[00:03] niemeyer: i'm sorry to say, but somehow you reviewde an old diff [00:06] davecheney: Uh oh.. [00:06] davecheney: Let me check it again [00:07] niemeyer: two secs, i'll run the test and push the latest branch [00:07] davecheney: Okay [00:13] niemeyer: done, https://codereview.appspot.com/6347044 [00:14] davecheney: I did not review an old version.. I replied to your comment, which means it sticks in the location where the first comment was made [00:15] davecheney: I said "This should be in a test by itself", and then provided details of how to do so neatly. You mentioned "Done", and I said "Apparently not.", because the latest version didn't have a test by itself [00:52] niemeyer: ok, that was my mistake. but I am now very confused [00:52] davecheney: What's up? [00:53] is there still an outstanding todo on that branch, or is it all resolved ? [00:58] davecheney: The outstanding items IMO are in that last review I sent [00:59] davecheney: We're clearly talking across each other, so I'm not sure about what's the status quo [00:59] davecheney: The items I mentioned seem valid to me, and you didn't implement nor object to them [01:00] niemeyer: with the exception of someone repairing the environment when the RemoveMachine fails, I belive I have addressed everything else [01:00] please correct me if i am mistaken [01:12] davecheney: davecheney: I said "This should be in a test by itself", and then provided details of how to do so neatly. You mentioned "Done", and I said "Apparently not.", because the latest version didn't have a test by itself [01:13] niemeyer: ok, i understand now, i read that as 'this should be in _the_ test itself' [01:13] I will redo [01:13] davecheney: As I mentioned there, this can use BootstrapOnce to avoid rebootstrapping [07:34] davecheney, fwereade_: mornin' [07:34] wrtp, heyhey [07:34] davecheney, if you're around, also heyhey [07:35] wazzup! [08:10] wrtp: thanks for your review [08:10] patchset 12 has tests that work [08:10] davecheney: np. [08:10] including a full mock using a PA running in process [08:10] davecheney: ah, cool. [08:10] I can repropose it as a followup CL [08:10] davecheney: but that went? [08:10] gustavo didn't like it [08:11] as it wasn't a real test [08:11] once this branch is merged i'll propose them again and we can discuss it some more [08:13] davecheney: hmm. seems pretty much like a real test to me. maybe there's something i missed. [08:14] davecheney: was that from gustavo's comment on line 23 here: https://codereview.appspot.com/6347044/diff2/1:5003/environs/ec2/suite_test.go?column_width=80 ? [08:20] wrtp: no, gustavo felt that testing against ec2 local test with a mocked PA didn't test anyhting [08:21] wrtp: it'll be easier for me to repropose after this branch is (finally) submitted [08:21] davecheney: that's a bit odd. especially as it means none of the testing code in this CL is exercised. [08:21] davecheney: anyway, go with what he says... [08:22] morning [08:24] howdy [08:26] anyway gents, i'm taking a break for a bit [08:26] i'll see you in a few hours for a pow wow [08:31] low on sleep last night, popping out for some sunshine [08:59] grar, work is beguiling and distracting; *really* popping out now [09:03] fwereade_: dcc pleas a bit sun, we've got only clouds here. [09:58] i've got to go for a dentist's appointment now. hopefully i'll be back in time for the meeting, even if i might have difficulty speaking. [10:08] moin. [10:10] Aram, heyhey [10:23] Moin Aram [11:01] Good mornings! [11:01] bonjour [11:01] niemeyer: Heya [11:02] niemeyer, heyhey [11:03] Is it party time? [11:03] yes please [11:03] who has the invite ? [11:04] I'll do it [11:05] Done [11:05] a minute.... [12:10] Lunchtime [12:12] meh, both T400 and T410 are overheated by the flash in G+ Hangouts. [12:12] * Aram needs a new laptop, maybe. [13:49] Aargh, sometimes too stupid. :O Returned from function after err == nil. *sigh* [14:29] Yay, tests are green, now remove some debugging statements and propose it. [15:20] So, https://codereview.appspot.com/6374047 is in. [15:57] Lunch time! [16:11] TheMue: i'm looking at it. for some reason i seem to be losing all initial messages to codereview (presumably you did propose it without -wip), so thanks for the heads up. [16:37] TheMue: ping [17:06] * niemeyer waves [17:08] * wrtp waves good night to niemeyer [17:08] wrtp: Are you heading off? [17:09] niemeyer: i have to, yes, sorry. [17:09] wrtp: No need to be sorry.. have a pleasant time there :) [17:09] niemeyer: just been looking through TheMue's firewall code. [17:09] wrtp: Oh, sweet [17:09] wrtp: How does it feel? [17:09] niemeyer: not entirely sure. [17:10] wrtp: Cool, have you made comments on it? [17:10] niemeyer: i'm toying with the idea that a more centralised approach might be simpler and easier to understand [17:10] niemeyer: only superficial comments so far [17:10] wrtp: Thanks for that.. I'll have a look later [17:11] niemeyer: anyway, still thinking, see ya tomorrow! [17:12] wrtp: Cheers! [17:28] Feeling some post-lunch sleepiness.. will lay down for a while to get through the rest of the day in better shape. [17:28] Back soon [19:16] Aram: ping [19:17] niemeyer: pong [19:17] Aram: Yo [19:17] hey [19:17] Aram: Just a sync up on this: "Fantastic, I never liked this, however, I only emulated what we have in state now." [19:17] yes [19:17] Aram: I don't think Machine.String returns "machine-%d" today, does it? [19:19] niemeyer: no, it doesn't, you were right. machine-%10d is the machinekey, which is used directly by state tests. [19:19] Aram: Aha, super [19:19] Aram: This is actually wrong too [19:20] Aram: There's no reason to not do what you're doing there [19:20] Aram: LGTM [19:21] niemeyer: thanks. [19:21] niemeyer: http://bazaar.launchpad.net/~gophers/juju-core/trunk/view/head:/state/state_test.go#L172 [19:21] that's the machinekey, isn't it? [19:24] Aram: Well, it is, but look at the actual operation going on there [19:24] Aram: zkConn.Children [19:24] Aram: It's ignoring the abstraction we put in place entirely [19:25] yes. [19:25] Aram: To be honest, I'd rather not have this kind of test at all, but then we have to guarantee correctness without poking at internals like this which may be tricky in some cases. [19:25] I agree. [20:28] Aram: https://codereview.appspot.com/6345056/ ready to go in too.. thanks! [20:29] niemeyer: thanks, I'll push it and the three others that are pending on after I submit the one I'm working ATM. [20:29] Aram: Sweet