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