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