| jw4 | anyone around to give a quick review? http://reviews.vapour.ws/r/388/ | 01:31 |
|---|---|---|
| === liam_ is now known as Guest12049 | ||
| === kadams54 is now known as kadams54-away | ||
| === kadams54 is now known as kadams54-away | ||
| === kadams54 is now known as kadams54-away | ||
| * thumper takes a deep breath and dives into the massive pile of work emails | 19:33 | |
| lazyPower | thumper: o/ | 20:19 |
| thumper | o/ lazyPower | 20:20 |
| lazyPower | hows my favorite kiwi? | 20:20 |
| jw4 | davecheney: just responded to your review... thanks by the way. I disagreed with both of your comments :) | 20:57 |
| menn0 | waigani: you still need a review of the settings stuff? | 21:06 |
| waigani | menn0: yes please: http://reviews.vapour.ws/r/368/ | 21:06 |
| menn0 | waigani: doing it now | 21:06 |
| waigani | thanks | 21:07 |
| menn0 | waigani: done | 22:14 |
| menn0 | wallyworld_: morning | 22:14 |
| waigani | menn0: thanks | 22:14 |
| wallyworld_ | hi | 22:14 |
| wallyworld_ | thanks for fixing that bug | 22:14 |
| menn0 | waigani: no problems | 22:15 |
| menn0 | thanks for merging it | 22:15 |
| wallyworld_ | np, was a bit of an experience to get it in 1.21 | 22:15 |
| menn0 | wallyworld_, waigani: duh. The "no problems" was intended for wallyworld_ | 22:15 |
| wallyworld_ | turns out other charm/repo stuff has to be backported too | 22:15 |
| menn0 | :) | 22:15 |
| menn0 | wallyworld_: yeah I saw that. there was another way around that. i initially had that test working without creating a new charm | 22:16 |
| menn0 | wallyworld_: by copying and monkeying the existing logging charm | 22:16 |
| waigani | menn0: np, I take it as a given there will be problems with my review ;) | 22:16 |
| wallyworld_ | all good, at least 1.21 ans trunk are a little less diverged | 22:16 |
| menn0 | waigani: nothing major, except a potential pre-existiing bug | 22:16 |
| menn0 | wallyworld_: do you want me to backport to 1.20 as well? | 22:17 |
| waigani | menn0: cool, thanks again, looking now | 22:17 |
| wallyworld_ | menn0: not sure, i was hoping we'd stop doing so much with 1.20 now that 1.21 is almost here | 22:17 |
| menn0 | wallyworld_: it's "fairly" unlikely that people will hit that bug but it's pretty bad when they do | 22:18 |
| wallyworld_ | yeah | 22:18 |
| wallyworld_ | i'll find out from curtis what our plans are | 22:18 |
| menn0 | wallyworld_: my inclination is to do the backport | 22:18 |
| menn0 | it shouldn't take long | 22:18 |
| wallyworld_ | ok, may as well then. i know dimiter has backported some network stuff too | 22:19 |
| menn0 | wallyworld_: k | 22:19 |
| menn0 | wallyworld_: I'll try and get it done today | 22:20 |
| wallyworld_ | no rush, not sure when 1.20 is getting a new release | 22:20 |
| wallyworld_ | 1.21 is the focus this week | 22:20 |
| menn0 | wallyworld_: ok | 22:31 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| waigani | menn0: when you have a moment: http://reviews.vapour.ws/r/368/ | 23:09 |
| menn0 | waigani: looking | 23:12 |
| menn0 | waigani: so you think that was a bug? | 23:13 |
| waigani | menn0: yes | 23:13 |
| menn0 | waigani: did you expand the unit tests to cover that scenario? | 23:13 |
| waigani | menn0: I dumped the db, and the key used to get the doc | 23:13 |
| waigani | menn0: you were right, good catch | 23:14 |
| waigani | menn0: no, good point | 23:14 |
| menn0 | waigani: so what's with the checkAddEnvUUIDToCleanupsIdempotent comment? is it supposed to be there? | 23:16 |
| waigani | menn0: no, i deleted it didn't I? It was just a reminder for me while working on the branch. | 23:17 |
| menn0 | waigani: nope it's still ther | 23:17 |
| waigani | 0_O | 23:18 |
| menn0 | didn't hit save? | 23:18 |
| waigani | ah right, ugh | 23:18 |
| waigani | yeah I'll delete it | 23:18 |
| waigani | AND save | 23:18 |
| waigani | menn0: so the test. Should we check that the search key matches the settingsKey format with a regex? | 23:20 |
| menn0 | waigani: I don't think so. You want to engineer something where the first transaction attempt fails so that bit of code gets hit | 23:22 |
| menn0 | waigani: and then make sure the alive test works as expected | 23:22 |
| waigani | menn0: right, got it | 23:22 |
| menn0 | perhaps make the fix and test changes in a separate PR if it's going to take a while? | 23:23 |
| menn0 | waigani: ^ | 23:23 |
| waigani | menn0: I'll have a poke around, considering it's nearly lunch, if my hunger wins out I'll do it in a followup PR | 23:24 |
| menn0 | waigani: not sure if you know but you can induce transaction failures using SetAfterHooks and SetBeforeHooks | 23:26 |
| waigani | menn0: ah nice, thanks | 23:26 |
| waigani | menn0: so code is currently called at bottom of TestSetCharm // SetCharm fails when the service is Dying. | 23:27 |
| waigani | menn0: but it's not a positive test | 23:28 |
| === kadams54 is now known as kadams54-away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!