| thumper | mwhudson: please? | 00:20 |
|---|---|---|
| lifeless | hmm, friday has no asiapac ocr | 00:21 |
| lifeless | thumper: why aren't you in the ocr rotation ? | 00:22 |
| thumper | lifeless: because it was decided a while back that team leads are exempt | 00:22 |
| thumper | :) | 00:22 |
| lifeless | mmm | 00:22 |
| thumper | 'cause we're busy | 00:22 |
| lifeless | self inflicted | 00:22 |
| lifeless | :P | 00:22 |
| thumper | :P | 00:22 |
| thumper | ✁☹ | 00:23 |
| lifeless | o/~ | 00:23 |
| mwhudson | thumper: hi, sure | 01:00 |
| thumper | https://code.edge.launchpad.net/~thumper/launchpad/xmlrpc-lp-name-resolution/+merge/34500 | 01:01 |
| thumper | mwhudson: thanks | 01:01 |
| mwhudson | why are code review diffs in proportional fonts at the moment? | 01:04 |
| lifeless | ! | 01:07 |
| lifeless | probably the ubuntu beta font changes | 01:07 |
| mwhudson | i don't have the font installed | 01:08 |
| mwhudson | i guess that might be it | 01:08 |
| lifeless | file a bug | 01:08 |
| mwhudson | ok | 01:09 |
| mwhudson | launchpad-code? | 01:09 |
| lifeless | mwhudson: can I borrow your brain for a minute | 01:15 |
| lifeless | mwhudson: launchpad-web perhaps | 01:15 |
| lifeless | mwhudson: xmlrpc timeout handling is either broken, or terribly confusing, or both | 01:16 |
| thumper | https://code.edge.launchpad.net/~thumper/launchpad/short-lp-name-for-private-branches/+merge/34509 is the last part | 01:38 |
| mwhudson | lifeless: xmlrpc timeouts _ought_ to be the same as everything else | 01:45 |
| mwhudson | i guess it's probably a separate config | 01:45 |
| lifeless | mwhudson: do you have, oh 60 seconds ? | 02:12 |
| mwhudson | lifeless: yeah | 02:13 |
| lifeless | http://bazaar.launchpad.net/~lifeless/launchpad/oops/revision/11435 | 02:13 |
| lifeless | incremental thing | 02:13 |
| lifeless | should be pretty self-explanatory | 02:13 |
| lifeless | this actually fixes the bug 'statements that hit the timeout policy are not in the statement log' | 02:14 |
| mwhudson | lifeless: looks good to me | 02:15 |
| lifeless | thanks! | 02:15 |
| mwhudson | thumper: sorry for the delay | 03:15 |
| mwhudson | thumper: the assertResolves docstring has become a bit mangled | 03:15 |
| thumper | mwhudson: nm | 03:15 |
| thumper | I'm running the tests through ec2 | 03:15 |
| thumper | and about to run and get more coffee beans | 03:15 |
| thumper | we have the wine for tonight :) | 03:15 |
| thumper | mwhudson: does the docstring need some attention | 03:16 |
| thumper | ? | 03:16 |
| thumper | I should take a look I guess | 03:16 |
| thumper | oversight that I've missed that | 03:16 |
| mwhudson | thumper: yeah, the summary line mentions arguments its no longer parsed and the next line doesn't make sense at the grammar level | 03:17 |
| mwhudson | sorry, not the next line | 03:18 |
| mwhudson | :param lp_path: The path the short lp alias path for the branch. | 03:18 |
| lifeless | maybe a matcher would help? | 03:25 |
| * thumper is back with coffee | 03:48 | |
| thumper | mwhudson: are you able to finish the reviews off? | 04:53 |
| thumper | mwhudson: these last two branches finish off something for our stakeholders :) | 04:53 |
| mwhudson | thumper: back now | 04:58 |
| mwhudson | thumper: one down | 05:01 |
| mwhudson | thumper: two down | 05:07 |
| thumper | mwhudson: you approved the proposal without approving the review (must fix that) https://code.edge.launchpad.net/~thumper/launchpad/short-lp-name-for-private-branches/+merge/34509 | 05:14 |
| lifeless | thumper: fix it? just make it automatic :) | 05:16 |
| thumper | lifeless: yes... I know | 05:18 |
| thumper | lifeless: it isn't fixed yet | 05:18 |
| lifeless | thumper: kk, had a scary moment where I thought you meant 'reject approval if there is no 'approve' review. | 05:19 |
| thumper | what? | 05:19 |
| mwhudson | oh ok | 05:19 |
| thumper | we have an implicit review approval through email | 05:19 |
| lifeless | thumper: I bound 'fix it' to the wrong action | 05:19 |
| thumper | it just doesn't do it through the web (yet) | 05:19 |
| mwhudson | thumper: done | 05:24 |
| thumper | mwhudson: thanks | 05:24 |
| lifeless | thumper: nice, sounds great | 05:26 |
| === henninge changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [henninge,henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | Good morning jtv! | 07:44 |
| jtv | hi henninge! Forgot to turn on pidgin today | 07:45 |
| jtv | Nobody on call? :( | 07:45 |
| jtv | On Call: - || reviewing: - || queue: [henninge,henninge,jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | 07:45 |
| henninge | jtv: no Abel yet, no ... ;( | 07:45 |
| henninge | jtv: let's trade reviews, then. | 07:46 |
| jtv | henninge: OK! | 07:46 |
| jtv | Which one do you want reviewed first? | 07:46 |
| henninge | jtv: sampledata depends on the other one. | 07:46 |
| === jtv changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | OK | 07:46 |
| henninge | jtv: thank you | 07:47 |
| jtv | Mine: https://code.edge.launchpad.net/~jtv/launchpad/bug-618393/+merge/34515 | 07:47 |
| henninge | jtv: mine did not stay under the limit like yours does ... Sorry | 07:47 |
| henninge | Claimed it already ;) | 07:48 |
| jtv | henninge: your cover letter looks like a good read—I'm almost sorry that it goes to me who already knows the background! | 07:49 |
| henninge | I know, such a waste .... | 07:49 |
| henninge | (just kidding) | 07:49 |
| henninge | jtv: that's ok, I like to do that anyway to wrap up a branch for myself. | 07:49 |
| jtv | I agree. There's also the historical record, and just the mental step of taking a step back and explaining. So I'm _almost_ sorry. :) | 07:50 |
| jtv | henninge: about the statistics checks you removed from translationmessage-views.txt: I think that wasn't so much a complicated way of testing that the submission succeeded, as a misplaced test for the statistics. An example of two ancient ills of our doctests: testing low-level things in too much detail (basically unit-testing in integration tests) and throwing different tests into the same scenario because it's the scenario, not t | 07:57 |
| henninge | (truncated) | 07:57 |
| jtv | An example of two ancient ills of our doctests: | 07:58 |
| jtv | testing low-level things in too much detail (basically unit-testing in integration tests) | 07:58 |
| jtv | and throwing different tests into the same scenario because it's the scenario, not the functionality, that dictates the structure of the test. | 07:58 |
| henninge | Well, my other thought was "statistics are tested elsewhere, no need to that here". | 07:59 |
| henninge | jtv: we'll be loosing codehosting in an hour, btw | 08:00 |
| jtv | Oh! Thanks for pointing that out. | 08:00 |
| jtv | Saw the email and didn't pay enough attention to it. | 08:01 |
| jtv | henninge: I think you did a good thing in removing this, and hopefully we can gradually cut down on doctests until we can actually figure out what they do and remove them completely. :) Please don't make the narrative say things like "Let's submit a translation" though. | 08:02 |
| henninge | sorry | 08:03 |
| henninge | I don't like that either, actually, but I was keeping in tone with the test as it was ... ;-) | 08:04 |
| henninge | 26-It's time to check the submission of translations and the IPOFile statistics | 08:04 |
| henninge | 27-update. | 08:04 |
| henninge | 28- | 08:04 |
| henninge | 29-But first, let's see current values. | 08:04 |
| jtv | Yes, it's horrible. | 08:04 |
| jtv | hi noodles775 btw—thanks for your review! Raises some difficult questions. | 08:06 |
| noodles775 | jtv: no problem... And yes, I found it hard :/ | 08:07 |
| jtv | There's a lot of complex background knowledge and intertwined data modeling there, and we've been reviewing a lot of this stuff among ourselves. | 08:08 |
| jtv | So perhaps a reminder to me that that is not always the case. :-) | 08:08 |
| jtv | I was a bit surprised that you got an impression of strong emotions from some of the comments in the code. Wasn't meant like that at all—saying "this record is in the way" just seemed like a good description of the situation. Is there anything there that you'd like me to change? | 08:09 |
| StevenK | thumper: I think you're EOW, but changes pushed for db-add-ifp-job. | 08:09 |
| jtv | If thumper is having his weekend now, maybe that's a good time to look at my wikkid MP. :) | 08:10 |
| noodles775 | jtv: no, not at all, I was just trying to be funny with that comment. | 08:17 |
| jtv | ah, phew! :) | 08:17 |
| jtv | I misread your facial expression then | 08:18 |
| noodles775 | Well yes, funny and sympathetic at the same time. | 08:18 |
| jtv | (emoticons are awesome… who needs a webcam for facial expressions) | 08:18 |
| jtv | noodles775: I'm engaged in mutual reviews with Henning right now, but will get back to it soon | 08:19 |
| noodles775 | Sure. | 08:19 |
| jtv | henninge: were we figuring out "this is a local suggestion" based on the pofile attribute rather than on which method call came up with the suggestion? | 08:21 |
| henninge | jtv: yes | 08:28 |
| jtv | may be a leftover from the caching we had before sharing… but glad to be rid of it! | 08:28 |
| jtv | I see you disabled unmasking in gettext-check-messages, but did not delete the code. Why is that? | 08:29 |
| * jtv remembers to check back with the excellent cover letter | 08:32 | |
| henninge | I thought I explained that ... ;) | 08:32 |
| henninge | jtv: We have a card to revisit that script and maybe we can come up with a way to re-enable it. | 08:32 |
| jtv | henninge: I appreciate the sentiment, but keeping commented-out code around is not usually a good idea. We have bzr to remember the old code for us! | 08:33 |
| jtv | henninge: I see now that the change you made is really quite minimal. That's nice in some ways, but if it leaves code dead, better to clean it up now. | 08:37 |
| jtv | Future work on the script will be easier if all the code that's in there actually works, and the "how did we use to do this" code is available separately in bzr history. | 08:37 |
| jtv | henninge_: you're blinking | 08:38 |
| === henninge_ is now known as henninge | ||
| jtv | henninge: daily router nap? | 08:38 |
| henninge | daily reconnect | 08:38 |
| henninge | yup | 08:38 |
| jtv | (14:37:01) jtv: henninge: I see now that the change you made is really quite minimal. That's nice in some ways, but if it leaves code dead, better to clean it up now. | 08:38 |
| jtv | (14:37:32) jtv: Future work on the script will be easier if all the code that's in there actually works, and the "how did we use to do this" code is available separately in bzr history. | 08:38 |
| henninge | I have to move that out of my working hours again. | 08:38 |
| jtv | I'm sure you can whip something up with "at" and "wget." :-) | 08:39 |
| henninge | jtv: I turn my computer off when I leave work ... | 08:40 |
| jtv | then leave it on for once, and set a shutdown time right after the "at" time! | 08:42 |
| jtv | hey danilos | 08:58 |
| danilos | jtv, hey hey | 08:58 |
| henninge | Hi danilos! ;) | 08:59 |
| henninge | jtv: | 09:24 |
| henninge | 133+ self._licenses = tuple([ | 09:24 |
| henninge | 134+ License.items[id] for id in sorted(license_ids)]) | 09:24 |
| henninge | henninge: | 09:34 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: - || reviewing: adeuring || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | hi adeuring! | 09:59 |
| adeuring | hi jtv | 10:00 |
| jtv | How's the weather in your neck of the woods? | 10:00 |
| adeuring | relatively cold and rainy | 10:00 |
| jtv | here too, but according to the Gnome weather applet the humidity still makes it _feel_ like 39.1° | 10:01 |
| * adeuring should try that too | 10:02 | |
| adeuring | is there also an option to feel snow during winter? | 10:03 |
| jtv | adeuring: not where I am | 10:03 |
| adeuring | henninge: which branch do you want reviewed? | 10:07 |
| jtv | adeuring: I'm working on one; the other depends on it | 10:08 |
| adeuring | jtv: ah, thanks! | 10:08 |
| henninge | adeuring: the sampledata branch is a simple sampledata change with many rows. | 10:10 |
| henninge | adeuring: I am not even sure it needs a review, though | 10:11 |
| adeuring | henninge: well, rs=me on the sampledata change | 10:11 |
| henninge | adeuring: thanks! | 10:11 |
| === henninge changed the topic of #launchpad-reviews to: On Call: - || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | henninge: The docstring for ensureBrowserPOFile is a bit ambivalent: it "ensures" and "makes sure" that the browser_pofile is set… "if possible." Which is it? | 10:24 |
| jtv | The only scenario I can imagine it coming up empty is where all templates that contained a potmsgset have been deleted. | 10:24 |
| jtv | Manually. | 10:24 |
| jtv | So maybe we simply shouldn't delete POTemplates without deleting orphaned potmsgsets. | 10:25 |
| henninge | jtv: See, I was not sure about that ... ;) | 10:25 |
| jtv | OTOH over a third of our potmsgsets seem to be orphans… | 10:26 |
| jtv | Maybe it's time to clean up some more there! | 10:26 |
| henninge | jtv: if they are orphans they won't come up in browser code, will they? | 10:27 |
| jtv | external suggestions. | 10:27 |
| jtv | henninge: they can still show up as external suggestions. | 10:37 |
| henninge | jtv: yes, I understand. I think that is the case where this used. | 10:46 |
| henninge | jtv: I think they should not be used for external suggestions, though. | 10:47 |
| henninge | So it will take an extra check to see if a browser_pofile could be found before displaying it. | 10:47 |
| jtv | henninge: yes, for the immediate term I think that'd solve it. | 10:52 |
| jtv | However it also means that 17% of our TMs become completely and utterly useless, even as external suggestions—so we should probably clean them up. | 10:52 |
| jtv | Minus the "probably." :-) | 10:52 |
| henninge | s/probably/definitly/ | 10:53 |
| henninge | ;-) | 10:53 |
| henninge | jtv: store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR) | 10:54 |
| henninge | Don't we have IStore now to replace that? | 10:54 |
| jtv | henninge: err yes, that'd be ISlaveStore. | 10:55 |
| henninge | jtv: I search and search but cannot really find any problems with your branch ... | 10:59 |
| jtv | henninge: try harder! :-) | 10:59 |
| * henninge gives it one more go | 11:00 | |
| jtv | Punish me babny! | 11:00 |
| jtv | (Sorry, it's hard to type correctly in the heat of passion) | 11:00 |
| henninge | jtv: although you did not introduce it, can you please fix this: | 11:01 |
| henninge | 59 # Prefill pillar.product.licenses. | 11:01 |
| henninge | 60- for pillar_name, other, product, project, distro, license_ids in ( | 11:01 |
| henninge | 61+ for pillar_name, other, product, project, distro, licenses in ( | 11:01 |
| henninge | 62 result[:limit]): | 11:01 |
| henninge | line 62 | 11:01 |
| jtv | I looked at it briefly but couldn't think of anything that'd make it fit. :) | 11:01 |
| jtv | I'll look again. | 11:01 |
| jtv | Grr circular import somewhere | 11:02 |
| henninge | jtv: you could pull the de-tupling into the loop... | 11:02 |
| henninge | jtv: Where? | 11:02 |
| jtv | Ah yes good | 11:02 |
| jtv | Not sure yet looks ugly | 11:02 |
| jtv | it says shipit | 11:03 |
| jtv | henninge: would it be convenient if ensureBrowserPOFile also returned self.browser_pofile? | 11:19 |
| henninge | no, I don't think so. | 11:19 |
| jtv | Well you've got a few cases that could benefit: ll.104 & 123 in the diff. | 11:21 |
| jtv | In other words, all call sites. :) | 11:21 |
| henninge | jtv: I am fine with that but it should still set browser_pofile because that may be used later in the code. | 11:24 |
| jtv | henninge: yes, hence the "also" :-) | 11:25 |
| henninge | jtv: review sent, r=me | 11:26 |
| jtv | henninge: cool, thanks! | 11:26 |
| jtv | Still trying to figure out what's wrong with the import error that seems to have nothing to do with me. | 11:26 |
| henninge | jtv: which branch? Mine or yours? | 11:27 |
| jtv | mine | 11:27 |
| jtv | But another one for you: in getTranslationMessages, the Coalesce line-wraps a bit awkwardly. How about putting it into a variable called e.g. applicable_template? | 11:28 |
| jtv | Then the query could just say "applicable_template == self.potemplate.id" | 11:28 |
| henninge | good idea | 11:29 |
| jtv | The really nice side effect is that the variable name introduces documentation without needing comment. | 11:31 |
| henninge | jtv: Will I see all you suggestions in the review or should I do them right now? | 11:32 |
| jtv | henninge: either way's fine for me, as long as I know that I'm not waiting for you to implement them before I go on to the next one. :) | 11:32 |
| jtv | So I'll move on. | 11:32 |
| henninge | jtv: I prefer waiting for the review and then do them all in one go. | 11:33 |
| jtv | In updateTranslation you pass pofile=None to the TranslationMessage constructor. It's a bit moot with the Recife changes coming up, but why not remove that argument altogether? | 11:33 |
| jtv | Oh, OK | 11:33 |
| henninge | jtv: I did not want to mess with the interface. But is pofile not used elsewhere in updateTranslation? | 11:34 |
| jtv | The argument, not the parameter. | 11:34 |
| jtv | I mean: there should be no need for the call site to pass pofile=None to the TranslationMessage constructor. | 11:34 |
| jtv | Should be the same as not passing pofile there at all. | 11:35 |
| henninge | that's fine | 11:35 |
| === bac changed the topic of #launchpad-reviews to: On Call: - || reviewing: adeuring, bac || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| bac | good morning adeuring | 11:52 |
| adeuring | hi bac! | 11:52 |
| bac | adeuring: busy day? | 11:52 |
| adeuring | bac: not at all :) | 11:52 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | henninge: review sent. | 12:04 |
| jtv | henninge: I've not received any review | 12:12 |
| * henninge checks | 12:12 | |
| jtv | Also, for all those who read German: http://www.nichtlustig.de/toondb/100901.html | 12:12 |
| henninge | jtv: hm, I have it in "Sent" ... | 12:13 |
| henninge | jtv: and I have received yours | 12:13 |
| jtv | henninge: I do not have yours in "Received," nor has the page updated. | 12:13 |
| henninge | I can resent it | 12:14 |
| henninge | resend | 12:14 |
| jtv | Maybe just use the web UI? | 12:14 |
| henninge | Well, it worked the other day. | 12:14 |
| henninge | jtv: done | 12:15 |
| henninge | I will go to lunch now. | 12:15 |
| jtv | henninge: and page updated. Thanks! | 12:16 |
| jtv | noodles775: getting back to your review now… I've implemented the smaller suggestions already, but now the same shipit problem is hitting me. :( | 12:47 |
| === matsubara-afk is now known as matsubara | ||
| bac | henninge: thanks for the sampledata update | 13:15 |
| noodles775 | jtv: I thought you mentioned re-merging devel brought the fix in? | 13:20 |
| jtv | noodles775: devel, yes. But this is a long-lived feature branch based off db-stable. :( | 13:20 |
| noodles775 | Ah, right. But that shouldn't effect whether the review continues should it? | 13:20 |
| noodles775 | (and won't you guys need to bring the same fix in anyway? either when it hits db-stable or manually?) | 13:21 |
| noodles775 | jtv: actually, it might be easier to wait until the fix hits db-stable and just merge. That'll give me a chance to try to get my own fix in before pqm closes too :) | 13:24 |
| jtv | noodles775: yes, I think so too | 13:25 |
| jtv | After all I can't easily test whether a particular change I make during the review is good. I'm basically blocked on this. | 13:25 |
| jtv | We'll have to merge a fresh db-stable eventually. | 13:25 |
| jtv | We do that periodically | 13:25 |
| === salgado is now known as salgado-brb | ||
| matsubara | adeuring, Hi Abel, could you review https://code.edge.launchpad.net/~matsubara/launchpad/update-ec2-merge-workflow/+merge/34486 for me please? | 14:01 |
| adeuring | matsubara: sure | 14:01 |
| matsubara | thanks adeuring | 14:01 |
| jtv | bac: very small one for review: https://code.launchpad.net/~jtv/launchpad/bug-629442/+merge/34539 | 14:16 |
| bac | jtv: ok | 14:16 |
| jtv | bac: thx | 14:17 |
| === bac changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: -, jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | bac: grrr slow diff—I'll poll for it to appear and then let you know | 14:19 |
| bac | jtv: no bother | 14:20 |
| jtv | bac: we have diff | 14:24 |
| jtv | repeat: we have diff | 14:24 |
| bac | thank you, houston | 14:24 |
| adeuring | matsubara: in lines 48..55 of the diff: if both (--no-qa or --incr) and --rollback=... are defined, the former options are ignored. Should we issue are warning in this case? | 14:27 |
| === salgado-brb is now known as salgado | ||
| matsubara | adeuring, you mean like raising an exception if --no-qa=True and -- incr=True and --rollback=123? | 14:30 |
| matsubara | adeuring, sounds like a good idea, yes | 14:31 |
| adeuring | matsubara: I don't know if this is worth an exception, just printing a user warning might be enough. But I leave decision to you ;) | 14:31 |
| bac | jtv: lovely, r=bac | 14:32 |
| jtv | thanks bac! | 14:32 |
| === bac changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| matsubara | adeuring, ok, I'll go with the exception as this is the same pattern used by the other options. | 14:32 |
| adeuring | matsubara: OK | 14:33 |
| adeuring | matsubara: in line 109, you catch a TypeError, probably raised in line 55, or somewhere in the option parser. But TypeErrors are quite generic -- could you move the "try: ... except TypeError:" directly to the place(s) where it occurs and raise the BzrComandError there? TypeError are so generic, and catching them not very close to the code where they are expected can hide completely unrelated errors. | 14:41 |
| matsubara | adeuring, just a sec, I'm on the stand up call | 14:46 |
| matsubara | adeuring, moving BzrCommandError to autoland.py wouldn't make much sense. I just tried something else instead and I think I can remove the exception handling | 14:51 |
| adeuring | matsubara: ok | 14:52 |
| matsubara | adeuring, since 67..71 I'm defining the --rollback option as int, if I pass something else like --rollback=foo, the option parser will return me a nice error message | 14:52 |
| matsubara | so, I'll remove the exception handling from there as it's unecessary in that case. | 14:53 |
| adeuring | matsubara: right, I wandered too if it was even necessary. (BTW, What happens for a missing parameter value) | 14:54 |
| matsubara | adeuring, this https://pastebin.canonical.com/36722/ | 14:55 |
| adeuring | matsubara: so, that's an optparse issue -- let's not worry about it. I was just curious if this might cause the type error in 'rolback=%d'' % rollbak | 14:57 |
| matsubara | adeuring, yep, I was being extra careful catching that TypeError as I thought 'rollback=%d'' % rollback' would raise that, but then optparse will take care of that for us. | 14:58 |
| adeuring | exaxtly | 14:59 |
| adeuring | matsubara: so, with "except TypeError:" removed and the other change about the options --no-qa and --rollback, r=me | 15:02 |
| matsubara | adeuring, I will push the branch with your review comments in a moment. thanks for the review! | 15:02 |
| danilos | adeuring, bac: got some time for a quick review? https://code.edge.launchpad.net/~danilo/launchpad/bug-548375/+merge/34544 | 15:08 |
| bac | danilos: yep! | 15:08 |
| adeuring | danilos: sure | 15:08 |
| === bac changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: -, danilos || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| bac | adeuring: i beat you to it | 15:08 |
| danilos | heh, I'll let you guys fight it over, though bac was first ;) | 15:08 |
| adeuring | bac: ok ;) | 15:08 |
| danilos | bac, oh, I just noticed there's one typo in the test (line 23): is_published=True should be is_published=is_published (use the passed in value: forgotten change after the unification of two tests) | 15:13 |
| bac | danilos: ok | 15:13 |
| === noodles775 changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: -, danilos || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| danilos | bac, most of the changes are drive-by or lint fixes, basically, just the tests and 3 lines I mention are specific to this bug fix; entire test_translation_importer.py is drive-by cleanups | 15:25 |
| bac | danilos: ok | 15:25 |
| noodles775 | Hi adeuring, bac. When you've time, could one of you please look at: https://code.edge.launchpad.net/~michael.nelson/launchpad/628711-generate-ppa-htacess-too-slow-try-2/+merge/34546 | 15:25 |
| adeuring | noodles775: sure | 15:25 |
| noodles775 | Thanks adeuring | 15:26 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: noodles, danilos || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: noodles, danilos || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| noodles775 | :) | 15:26 |
| bac | danilos: done. thanks. | 15:29 |
| === bac changed the topic of #launchpad-reviews to: On Call: adeuring, bac || reviewing: noodles, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| danilos | bac, cheers | 15:31 |
| jelmer | hi adeuring, bac | 15:48 |
| bac | hi jelmer | 15:49 |
| jelmer | Can I add another Soyuz branch to the queue? | 15:49 |
| jelmer | It's at https://code.edge.launchpad.net/~jelmer/launchpad/506256-remove-popen/+merge/34549 | 15:49 |
| adeuring | bac: could you take the review? I'm busy with noodles's review and some other issues | 15:49 |
| bac | adeuring: sure | 15:49 |
| bac | jelmer: sure | 15:49 |
| noodles775 | adeuring: is it worth me sending my branch off to ec2 test (not to land) or can you already see changes needed? | 15:50 |
| noodles775 | (so I can get in before pqm closes) | 15:50 |
| bac | jelmer: i just reviewed your tiny branch. will start on the other one now. | 15:59 |
| jelmer | bac: thanks | 16:00 |
| bac | jelmer: i get this: | 16:04 |
| bac | from testtools.content_type import UTF8_TEXT | 16:04 |
| bac | ImportError: cannot import name UTF8_TEXT | 16:04 |
| bac | jelmer: is this a new dependency? | 16:04 |
| jelmer | bac, I've seen that before as well, but it's not from my branch | 16:04 |
| jelmer | bac: have you updated sourcedeps recently? | 16:05 |
| bac | this morning | 16:05 |
| bac | make tea, shave, update rocketfuel. | 16:05 |
| bac | ok, so it's not that routine but i did update this a.m. | 16:06 |
| jelmer | hmm | 16:07 |
| bac | trying your tests on devel | 16:07 |
| bac | jelmer: thinking about your LPCONFIG change....perhaps you should run it by the LOSAs, just so they will know about the change | 16:11 |
| bac | and see if they have comments. it does affect the way they start things | 16:11 |
| jcsackett | bac: i got that error once (UTF8_TEXT) post-update and make clean && make build fixed it. | 16:11 |
| bac | jcsackett: that's what i'm doing now. good to know. | 16:12 |
| jelmer | bac: That's a good idea - thanks | 16:12 |
| bac | jelmer: it's not good to surprises the losas... | 16:12 |
| adeuring | noodles775: sorry, I was somewhat distracted by other issues... In exportSubscriptions(), can't you do the equivalent of all_activre_tokens.difference(...) in SQL? | 16:24 |
| noodles775 | adeuring: I'm not sure what you're asking? That is being executed as one SQL statement? | 16:25 |
| adeuring | noodles775: argh, yes! | 16:26 |
| adeuring | sorry for the noise | 16:26 |
| noodles775 | np! Always good to ask :) | 16:26 |
| bac | jelmer: you mention a failing test. is it ./bin/test lp.buildmaster ? I'm getting ForbiddenAttribute, among other failures | 16:29 |
| jelmer | bac: no, it should be in lp.archiveuploader | 16:29 |
| jelmer | "./bin/test lp.buildmaster" is passing for me here - what's the exact error you're getting? | 16:29 |
| adeuring | noodles775: seems that I am constantlxy distracted by some other (urgent) issues -- would you mind if I put your review back into the queue? | 16:31 |
| noodles775 | adeuring: no, I understand. Hopefully bac will have some head-space :) | 16:32 |
| adeuring | noodles775: thanks! | 16:32 |
| === adeuring changed the topic of #launchpad-reviews to: On Call: bac || reviewing: -, - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === Ursinha is now known as Ursinha-lunch | ||
| noodles775 | bac: when you're free, it's this one (which I'm hoping to get in before pqm closes ;)): https://code.edge.launchpad.net/~michael.nelson/launchpad/628711-generate-ppa-htacess-too-slow-try-2/+merge/34546 | 16:33 |
| bac | noodles775: i'll have a look when i get done with jelmer's | 16:34 |
| noodles775 | Thanks bac | 16:35 |
| noodles775 | bac: actually, I've just found an issue with my branch while doing some QA, so please don't start it until I ping you again :) | 16:49 |
| bac | noodles775: ok | 16:49 |
| bac | brb | 16:53 |
| === salgado is now known as salgado-lunch | ||
| henninge | gmb: still around? | 17:07 |
| gmb | henninge, Yes | 17:08 |
| henninge | gmb: what do I need to do for a pre-emptive r-c? Add you for an r-c review? | 17:08 |
| gmb | henninge, Yes please. | 17:08 |
| === gmb changed the topic of #launchpad-reviews to: On Call: bac || reviewing: -, - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM for RC reviews please add him as a reviewer | ||
| === gmb changed the topic of #launchpad-reviews to: On Call: bac || reviewing: -, - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| gmb | </punctuationpendantry> | 17:09 |
| henninge | gmb: https://code.edge.launchpad.net/~henninge/launchpad/devel-bug-597359-sampledata/+merge/34444 | 17:10 |
| * gmb looks | 17:10 | |
| henninge | gmb: I am waiting for the prerequisite branch to make it through ec2 but I am not sure that I can watch it closely enough tonight to get the timing right. ;-) | 17:10 |
| bac | jelmer: here are the test failures i'm seeing: http://pastebin.ubuntu.com/487888/ | 17:12 |
| jelmer | bac: the testNoFiles one is what I'm aware of, I'll try to reproduce the other two. | 17:13 |
| gmb | henninge, rc=me | 17:18 |
| henninge | gmb: thanks! | 17:18 |
| === matsubara is now known as matsubara-lunch | ||
| noodles775 | bac: Mine is ready to go now (I've push the small change and already QA'd it on dogfood) | 17:19 |
| bac | noodles775: is 11492 the latest? | 17:20 |
| bac | that's what the MP shows | 17:20 |
| noodles775 | bac: 11493 is what I've push 10mins ago. | 17:21 |
| bac | noodles775: ok, i'll wait | 17:21 |
| noodles775 | bac: its displaying for me? | 17:21 |
| bac | jelmer & noodles775: i'm working to get your reviews done. unfortunately i have to leave in 1 hour | 17:21 |
| noodles775 | gmb: I've added you for a pre-emptive r-c to https://code.edge.launchpad.net/~michael.nelson/launchpad/628711-generate-ppa-htacess-too-slow-try-2/+merge/34546 | 17:22 |
| bac | noodles775: see it now | 17:22 |
| noodles775 | (if you've time) | 17:22 |
| noodles775 | Thanks bac | 17:22 |
| === Ursinha-lunch is now known as Ursinha | ||
| gmb | noodles775, Looking now. | 17:24 |
| bac | noodles775: my gem today is defaultdict. thanks! | 17:25 |
| noodles775 | :) | 17:25 |
| bac | noodles775: looks good to me. r=bac | 17:26 |
| noodles775 | Thanks bac | 17:26 |
| === bac changed the topic of #launchpad-reviews to: On Call: bac || reviewing: jelmer || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| noodles775 | gmb: I need to run, but I'll be checking back later to land this branch. I've just updated the related bug with the QA on dogfood. Thanks! | 17:29 |
| gmb | noodles775, np | 17:29 |
| === benji is now known as benji-lunch | ||
| bac | jelmer how's it coming? | 17:49 |
| * bac finds quick lunch | 17:49 | |
| jelmer | bac: So, I can reproduce the errors locally but I don't see how I didn't get them on df | 17:49 |
| jelmer | bac: I'm still investigating | 17:50 |
| bac | jelmer: ok | 17:50 |
| bac | jelmer: i've got about 30 minutes left. EdwinGrubbs has agreed to cover last minute submissions after that. | 17:50 |
| jelmer | bac: ok, thanks | 17:51 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On Call: bac, Edwin || reviewing: jelmer || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| === matsubara-lunch is now known as matsubara | ||
| === sinzui changed the topic of #launchpad-reviews to: On Call: bac, Edwin || reviewing: jelmer || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| sinzui | EdwinGrubbs, , bac: I have a oops fix for answers: https://code.edge.launchpad.net/~sinzui/launchpad/question-title-0/+merge/34555 <- I made that in the UI because my email does not seem to have arrived. | 18:07 |
| EdwinGrubbs | sinzui: I'll take it. | 18:08 |
| === salgado-lunch is now known as salgado | ||
| === bac changed the topic of #launchpad-reviews to: On Call: Edwin || reviewing: sinzui || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| bac | jelmer: i need to dash. | 18:19 |
| jelmer | bac: Ok | 18:20 |
| bac | EdwinGrubbs: i owe you a Lone Star in dallas. | 18:20 |
| bac | maybe a pearl | 18:20 |
| jelmer | bac: Thanks for having a look. | 18:20 |
| EdwinGrubbs | sinzui: r=me | 18:31 |
| sinzui | EdwinGrubbs, thanks | 18:33 |
| === matsubara is now known as matsubara-dr | ||
| === benji-lunch is now known as benji | ||
| === james_w` is now known as james_w | ||
| === deryck is now known as deryck[lunch] | ||
| === matsubara-dr is now known as matsubara | ||
| === deryck[lunch] is now known as deryck | ||
| === _thumper_ is now known as thumper | ||
| === matsubara is now known as matsubara-afk | ||
| === salgado is now known as salgado-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!