wallyworld | tlm: i have a couple of questions about the watcher PR, you free for a HO? | 02:21 |
---|---|---|
tlm | wallyworld: yep | 02:22 |
wallyworld | looks like hpidcock raised one of my issues already | 02:22 |
tlm | does hpidcock want to join ? | 02:22 |
wallyworld | let's jump standup | 02:22 |
hpidcock | kk | 02:23 |
babbageclunk | wallyworld: when you get a moment can you take a look at https://github.com/juju/juju/pull/11160? It's the CMR offer name fix | 03:40 |
wallyworld | babbageclunk: yeah, was reviewing then had to go to cataylst meeting | 03:42 |
wallyworld | will finish when meeting is done | 03:42 |
babbageclunk | wallyworld: oh right - thanks! | 03:44 |
wallyworld | babbageclunk: i'm having trouble groking names.NewApplicationTag(offerName), maybe we can discuss. i just need to be educated | 04:00 |
babbageclunk | wallyworld: it's the other side of this line: https://github.com/juju/juju/blob/develop/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go#L311 | 04:02 |
wallyworld | babbageclunk: just witj john in leads call, free free to jump in | 04:03 |
babbageclunk | wallyworld: it's definitely weird though - and caused me a bit of trouble (since it makes the tags coming back from looking up remote entities ambiguous | 04:03 |
babbageclunk | wallyworld: wilco | 04:03 |
babbageclunk | ) | 04:04 |
wallyworld | tlm: i left a nitpic plus a cleanup suggestion | 05:36 |
wallyworld | hpidcock: was https://bugs.launchpad.net/juju/+bug/1855777 fixed do you recall? | 07:14 |
mup | Bug #1855777: Seg fault generated due to malformed bundle.yaml file <bundles> <ux> <juju:Triaged by hpidcock> <https://launchpad.net/bugs/1855777> | 07:14 |
wallyworld | there was another bundle fix that got done | 07:15 |
wallyworld | we may have done both | 07:15 |
hpidcock | wallyworld: I'll verify, but I believe so | 07:15 |
wallyworld | ta, just doing gardening for 2.7.2 RC | 07:15 |
hpidcock | when did 2.7.1 go out?? | 07:18 |
wallyworld | last week at sprint | 07:18 |
hpidcock | ok | 07:18 |
wallyworld | it's ok if we mark against 2.7.2 | 07:18 |
wallyworld | so long as it goes fix committed | 07:18 |
wallyworld | ty | 07:19 |
hpidcock | yeah they both were done atm tlm's induction | 07:20 |
hpidcock | sorry, missed the part about marking as committed | 07:20 |
=== parlos is now known as parlos_afk | ||
=== parlos_afk is now known as parlos | ||
=== parlos is now known as parlos_afk | ||
=== parlos_afk is now known as parlos | ||
stickupkid | manadart, I'm going to try and create a PR for CMR in 2.7... this will be fun | 09:20 |
manadart | stickupkid: Yay. Have you looked at babbageclunk's CMR patch? I haven't checked if that will get in our way yet. | 09:21 |
stickupkid | manadart, not at all, will check now | 09:22 |
stickupkid | manadart, I like that he's have his own conversation in this comment https://github.com/juju/juju/pull/11160/files#diff-cf3dd42d097ceeee567e5127f9a1e1abR190-R196 | 09:23 |
=== parlos is now known as parlos_afk | ||
stickupkid | manadart, one thing we've not done is removed the restriction + feature flag | 09:39 |
manadart | stickupkid: I think we have sufficient confidence and test coverage to do that. | 09:40 |
stickupkid | I'll do it after the merge | 09:40 |
=== parlos_afk is now known as parlos | ||
stickupkid | manadart, if I die and so I don't have to repeat this process - I believe these are the CMR commits - going to re-check now https://paste.ubuntu.com/p/rDZ8TN99jS/ | 10:04 |
nammn_de | manadart: in for another cr? https://github.com/juju/juju/pull/11143 I reworked the code to use modeloperation | 11:20 |
manadart | nammn_de: Few items remaining from my last review. | 11:25 |
nammn_de | manadart: oh, really? Sorry. Gonna fix them, thought I resolved them | 11:25 |
nammn_de | manadart: ahh damn my ide refactor refactored more places than i wanted | 11:26 |
nammn_de | dammnit | 11:26 |
=== parlos is now known as parlos_afk | ||
nammn_de | ahhhh I just realized that you reviewed before I asked and buff got confused =D | 11:31 |
=== parlos_afk is now known as parlos | ||
nammn_de | manadart: regarding 2 of your comments: | 11:58 |
nammn_de | - doesnt need to that interface if it doesnt implement. But didnt it? But I think it didn't had any usage so I just removed it and the function using it now uses state.ModelOperation instead | 11:58 |
nammn_de | - A *state.State supplies it directly.: the api doesn't use a *state.State in its struct. Should I add one? I thought we didn't want that | 11:58 |
nammn_de | manadart: I added some comments/questions to those PR comments from you. Thanks! | 12:05 |
manadart | nammn_de: Replied to your comment. | 12:21 |
=== parlos is now known as parlos_afk | ||
=== parlos_afk is now known as parlos | ||
nammn_de | manadart: maybe I am wrong, but deleting it stills fails compilation for me: apiserver/facades/client/spaces/spaces_rename.go:235:23: api.backing.ApplyOperation undefined (type Backing has no field or method ApplyOperation) | 12:56 |
manadart | nammn_de: Applying this to your branch builds fine. https://pastebin.canonical.com/p/rMH88bWKYj/ | 13:02 |
nammn_de | manadart: damnit stupid me, thanks! I removed from the interface and from the shim | 13:02 |
nammn_de | manadart: pushed the changes and applied your review. | 13:06 |
achilleasa | manadart: so, the HookContext uses an interface and gomock but the NewHookContext in export_test uses concrete instances :D | 13:09 |
achilleasa | I might be able to cheat and get my tests in without messing with embedding uniter.Unit | 13:11 |
achilleasa | manadart: doh! hml introduced a mockHookContextSuite which I totally missed (other file). I will add my tests there :-) | 13:31 |
manadart | achilleasa: Standup. | 13:31 |
=== Guest49404 is now known as skay | ||
achilleasa | hml: can you please review https://github.com/juju/juju/pull/11164? | 14:17 |
hml | achilleasa: sure | 14:18 |
hml | achilleasa: approved. | 14:42 |
achilleasa | hml: tyvm | 14:43 |
=== salmankhan1 is now known as salmankhan | ||
stickupkid | manadart, I think I solved the CMR git diff issue, i didn't start far back enough | 15:09 |
manadart | stickupkid: Yeah, I think there were some prep commits quite a while ago. | 15:09 |
achilleasa | hml: did you see my reply to your comment? Should I go ahead and merge? | 15:34 |
hml | achilleasa: not yet, looking | 15:34 |
hml | achilleasa: agreed, i figured a small chance, worth noting, but not worth a different approach. | 15:36 |
hml | merge away | 15:36 |
=== salmankhan1 is now known as salmankhan | ||
nammn_de | manadart: time for a call later today or tomorrow to spec out remove-space/if needed rename-space, a little? | 15:46 |
manadart | nammn_de: Sure. I have to head home now, but ping me when you are on tomorrow. | 16:10 |
nammn_de | manadart: sure will do =D. Nice evening! | 16:10 |
stickupkid | I know what's wrong, I'm missing a PR | 16:27 |
rick_h | stickupkid: doh! easy fix? | 16:35 |
stickupkid | starting again, it's quite far back :( | 16:35 |
rick_h | stickupkid doh | 16:37 |
stickupkid | rick_h, this is the new list https://paste.ubuntu.com/p/2GKNYPW5tm/ | 16:37 |
* rick_h is afraid to click | 16:37 | |
stickupkid | it's ok till about 1/2 way in and then explodes | 16:37 |
stickupkid | one last go, before I try again tomorrow | 16:38 |
rick_h | oh geeze... | 16:38 |
stickupkid | the fixes aren't hard, but I'm obviously missing a PR somewhere | 16:38 |
stickupkid | brb grabbing a drink | 16:38 |
stickupkid | rick_h, it's babbageclunk changes are causing a bit of an issue - this was to be expected, but I've managed to get further now than i have done, so we're close | 17:26 |
stickupkid | rick_h, done it! | 17:33 |
stickupkid | rick_h, nice healthy set of changes +8,096 −1,523 | 17:38 |
rick_h | stickupkid: woot woot | 17:44 |
rick_h | stickupkid: congrats! | 17:44 |
stickupkid | there are some issues with dependencies, but I'll solve them tomorrow, seems the gopkgs.toml file isn't in sync | 17:45 |
rick_h | stickupkid: cool yea and guild please help stickupkid with any testing/QA so we can make sure we're feeling good about it in 2.7.2 | 17:45 |
rick_h | stickupkid: maybe reach out to babbageclunk to help as well in his TZ since he's in there a little bit | 17:45 |
rick_h | #moar-eyes-better | 17:46 |
babbageclunk | rick_h: what's the problem? | 20:36 |
babbageclunk | rick_h: ah - has there been a decision to release cmr migration in 2.7? | 20:40 |
babbageclunk | that 8k line PR looks fun | 20:43 |
rick_h | babbageclunk: it's a backport of the cmr migration bits to 2.7 yea | 20:53 |
babbageclunk | ouch | 20:54 |
wallyworld | rick_h: backporting a rather large feature to 2.7 seems risky? | 21:19 |
rick_h | wallyworld: we're hanging out in the team chat we can discuss it if you're up for it | 21:20 |
addyess | i'd apreciate a review of charm-helpers PR: https://github.com/juju/charm-helpers/pull/423 | 22:22 |
addyess | thanks timClicks | 22:35 |
techalchemy | addyess, thanks for putting that ll together it looks good to me fwiw | 23:11 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!