/srv/irclogs.ubuntu.com/2009/12/14/#launchpad-reviews.txt

thumpermwhudson: https://code.edge.launchpad.net/~thumper/launchpad/fix-windmill-reply-test/+merge/16106 one line fix for the windmill test00:17
* mwhudson waits for the diff00:18
thumperit's there00:24
=== adiroiban changed the topic of #launchpad-reviews to: on call: || reviewing: - || queue [adiroiban(bug-435165),adiroiban(bug-496352)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
StevenKAwww, no on call reviewer. Damn this Australian timezone!04:47
mwhudsonStevenK: i can probably do it for you if it's simple04:48
StevenKmwhudson: It's a patch change for a cronscript that is run by the publisher04:49
mwhudsonStevenK: let's see it04:49
mwhudsonStevenK: i may run away screaming, but i'll take a loot04:49
mwhudsonlook04:49
StevenKmwhudson: I'll be running it past cjwatson when he wakes, but a second pair of eyes is what I wanted04:49
StevenKmwhudson: http://paste.ubuntu.com/340938/ ; in leiu of pushing it as a branch yet04:50
mwhudsonStevenK: looks fine to me, but hardly my area of expertise04:51
StevenKmwhudson: I'll throw it past Colin, and push it as a branch if he says go, if that sounds fine04:52
mwhudsonStevenK: i don't see why you couldn't push it as a branch now04:53
StevenKPoint.04:53
* StevenK pushes04:53
mwhudsonand propose a merge04:53
mwhudsonStevenK: once colin has said ok, you should be able to find an eu-timezoned dev to approve it easily enough04:54
mwhudsonStevenK: is this urgent?04:54
StevenKmwhudson: It's a germinate change, so the sooner the better04:55
mwhudsonStevenK: because launchpad is being released on thursday, we're in pre-release freeze04:55
mwhudsondanilo[home] is the man to convince to get it past the freeze04:55
StevenKmwhudson: My commit message won't appear in PQM's message, since I've noticed in bzr log they have "special" formatting04:55
StevenKSigh, that's supposed to be a question04:56
mwhudsonStevenK: that's ok, we have magic to add the "specialness" now04:56
StevenKmwhudson: The Reviewer should be 'launchpad' ?04:59
mwhudsonStevenK: the default should be fine04:59
StevenKmwhudson: In that case, I've pushed the branch, and proposed it for merging05:00
mwhudsonStevenK: cool05:00
StevenKExcept that Launchpad just OOPS'd on it05:00
mwhudsonStevenK: you can request a review from colin too05:00
mwhudsonStevenK: :(05:00
mwhudsonStevenK: what was the oops?05:00
StevenKOOPS-1444EA77905:00
mwhudsonoh except the log syncing is disabled05:02
StevenKmwhudson: Shall I try it again, or you're looking/added it to a list to be looked at?05:05
mwhudsonStevenK: definitely worth trying again05:05
mwhudsonStevenK: it was a timeout, which is a bit odd, but try again indeed05:06
StevenKAnd it worked \o/05:06
mwhudsonyay05:09
jmlhi05:53
=== jpds changed the topic of #launchpad-reviews to: on call: || reviewing: - || queue [adiroiban(bug-435165),adiroiban(bug-496352,jpds(bug-176396))] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue [adiroiban(bug-435165),adiroiban(bug-496352,jpds(bug-176396))] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== danilo[home] is now known as danilos
=== matsubara-afk is now known as matsubara
intellectronicahenninge: i haven't added my name because i'm chr and also catching up with stuff after a week and a bit of absence. but if things get busy by all means ask me to help out12:16
henningeintellectronica: that's fine. Enjoy your CHR week! ;)12:17
intellectronica:D12:18
=== henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: adiroiban(bug-435165) || queue [adiroiban(bug-496352,jpds(bug-176396))] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
* henninge lunches first, though.12:49
=== henninge is now known as henninge-lunch
al-maisanhenninge-lunch: I'd appreciate it if you could review this simple branch as well: https://code.edge.launchpad.net/~al-maisan/launchpad/disable-ppa-495975/+merge/1612513:00
=== al-maisan changed the topic of #launchpad-reviews to: on call: henninge || reviewing: adiroiban(bug-435165) || queue [adiroiban(bug-496352,jpds(bug-176396)),al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisanhenninge-lunch: .. and BTW, we plan to get an RC for the branch and it's tiny .. so, please give it preferential treatment if possible. Thanks!13:02
=== mrevell is now known as mrevell-lunch
danilosStevenK, bigjools: I assume we will want an RC for https://code.edge.launchpad.net/~stevenk/launchpad/netbook-cron/+merge/16115? bigjools, if so, can you please review it first?13:26
bigjoolsdanilos: ok13:27
bigjoolsdanilos: approved13:28
danilosbigjools, cool, who will be landing this if I RC it? StevenK, can you land it or someone from the LP team needs to do it?13:28
bigjoolsdanilos: it will need to be one of us13:29
bigjoolsdanilos: maybe you :)13:29
bigjoolsthere are no tests, just pqm-submit it13:29
danilosbigjools, I have so many things on my hands right now that I am afraid I can't do it :)13:30
bigjoolsjoin the club :/13:31
danilosbigjools, I am IN the club already!13:31
bigjoolsdanilos: ok I'll do it, mark it r-c though will you?13:31
danilosbigjools, sure13:32
danilosbigjools, done13:33
bigjoolscheers13:33
=== mrevell-lunch is now known as mrevell
=== flacoste_afk is now known as flacoste
=== henninge_ is now known as henninge
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [adiroiban(bug-496352,jpds(bug-176396)),al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
allenapabentley: Fancy a quick OOPS fix to review? https://code.edge.launchpad.net/~allenap/launchpad/bugtracker-snapshot-bug-447100/+merge/1613014:33
abentleyallenap: I can add it to the queue.14:34
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [adiroiban(bug-496352,jpds(bug-176396)),al-maisan, allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
allenapabentley: Thanks.14:35
abentleyadiroiban: I notice you deleted "lp.soyuz.interfaces.archive import ArchivePurpose" from lib/lp/archiveuploader/uploadpolicy.py.  Was this an unneeded import?14:47
adiroibanabentley: yes. lint was complaining about it14:48
abentleyCool.  Thanks for fixing lint errors as you go.14:48
=== al-maisan changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [adiroiban(bug-496352,jpds(bug-176396)), allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adiroibanI wanted to fix the other errors ... withoud adding ignore comments... but I don't know how to fix them14:49
abentleyadiroiban: I don't know how to fix the F0401 for lazr.* without ignore comments.  For the email ones, you could try using the lowercase module names, e.g. "email.encoders"14:54
adiroibanok14:55
abentleyadiroiban: For email.MIMEText, it looks like the new name is email.mime.text.14:57
adiroibanabentley: thanks. I'll fix that14:59
=== matsubara is now known as matsubara-lunch
abentleyadiroiban: approved.  We won't be able to land it until next week because this is a release week.15:02
adiroibanabentley: no hurry. Thanks!15:03
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [jpds(bug-176396)), allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
abentleyadiroiban: Could you remind me next week, please.  And maybe run the full test suite in advance?15:03
adiroibanabentley: yes15:04
abentleyadiroiban: Great, thanks.15:04
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), jpds(bug-176396)) || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
abentleyjpds: Have you run the distributionmirror tests with your change?15:07
jpdsabentley: Yes.15:08
jpdsabentley: And I've tested it on a local Launchpad instance.15:09
abentleyjpds: Normally, I would expect that change to break a test.  Since it didn't, it means this behaviour isn't being tested.  You could add a test for it, or I can if you prefer.15:10
jpdsabentley: http://pastebin.ubuntu.com/341226/ ?15:16
abentleyjpds: Exactly.15:17
jpdsabentley: OK, let me test it locally and then I'll push.15:17
abentleyjpds: Sounds good.15:17
jpdsabentley: How strange: http://pastebin.ubuntu.com/341229/15:20
jpdsAh, wait, there's already a test there...15:21
jpdsabentley: My mistakes, pushed new revision with test.15:23
henningeadiroiban: do you have a good example on launchpad.dev for the template list?15:25
adiroibanhenninge: yes. I'm done :)15:25
henningeadiroiban: sorry, I am talking about bug 43516515:26
mupBug #435165: Make it easier to navigate to the full list of templates in source packages <trivial> <ui> <Launchpad Translations:In Progress by adiroiban> <https://launchpad.net/bugs/435165>15:26
adiroibani'm just toggling the unseen class15:26
henningewhat I am just reviewing15:26
adiroibanah15:26
abentleyjpds: Thanks.  Approved.  Since this is a release week, it can't land until we reopen for development.15:26
henningeETOOMUCHCONCURRENCY ;)15:26
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), allenap || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adiroibanhenninge: I have created some templates in the stories15:27
adiroibanalso I set the SHOW_RELATED to 115:27
henningeadiroiban: also, you have merge conflicts in the diff ?!?15:28
adiroibanand everhing was ok15:28
jpdsabentley: Thanks, when do we reopen?15:28
henningejpds: Friday night. Late, usually15:29
adiroibanhenninge: uhh... I'll do15:31
henningeadiroiban: xx-potemplate-index.txt is missing when I get your branch. That's where the conflict is.15:31
henningeadiroiban: yes, you'll have to fix that first.15:32
adiroibanhenninge: xx-rosetta-potemplate-index.txt was renamed to xx-potemplate-index.tx15:32
=== salgado is now known as salgado-lunch
henningeadiroiban: yes ...?15:33
adiroibanyes.15:33
adiroibanin another branch that was recently merged15:34
adiroibani did that15:34
henningeadiroiban: I mean, I never mentioned xx-rosetta-potemplate-index.txt15:34
henningeadiroiban: something is wrong with your branch then. Have a look at the diff on the MP.15:34
adiroibanhenninge: anyway, let me fix the branch and then you can review it15:35
adiroiban:)15:35
abentleyallenap: I may have been offline when I posted, so: "Could you explain the import reordering to me?  it's just that it seems to go against PEP8's recommendation of '1. standard library imports, 2. related third party imports, 3. local application/library specific imports'"15:50
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adiroibanhenninge: I have merged with devel and fixed that conflict15:58
adiroibansorry for the inconveniences15:59
allenapabentley: One of us was offline I think :) I think I should put lazr.lifecycle above the lp and canonical imports. The lazr.lifecycle import is new.15:59
abentleyallenap: I agree.  Approved, with that change.16:01
allenapabentley: Thanks :)16:01
=== matsubara-lunch is now known as matsubara
=== noodles775 changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775Hi henninge, abentley: if either of you has time: https://code.edge.launchpad.net/~michael.nelson/launchpad/475808-unembargoing-oops/+merge/1614116:10
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), noodles775 || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
abentleynoodles775: I suspect your F0401 lint errors can be fixed by changing the imports to email.mime.multipart and email.mime.text.  Can you check that?16:14
noodles775abentley: will do, thanks.16:14
=== salgado-lunch is now known as salgado
abentleynoodles775: On line 163 of the patch, you want "whose", not "who's".16:19
noodles775yep, btw, that fixed the lint, thanks!16:19
abentleynoodles775: Cool.16:20
abentleynoodles775: Also, you want "its", not "it's" on line 172.16:21
noodles775gah.16:21
abentleynoodles775: On the bright side, the code looks pretty good.16:22
abentleynoodles775: So for the case where there's no ancestor in the target of the copy, is that a valid case?  Or should we require the first upload to be done without copying?16:23
noodles775abentley: it's valid. Anyone can upload a pkg to their PPA (and we'll just override the component to 'main')16:25
noodles775It's just when copying that package to a non-ppa archive, we need to ensure the component will be overridden correctly.16:26
abentleynoodles775: Okay, can you explain why test_do_delayed_copy_wrong_component_no_ancestry should raise an exception?16:27
noodles775Well, I didn't change that behavior - any copy for an invalid component raises an exception currently. I'm just otp, but after the call I'll try to give a better answer.16:29
adiroibanabentley: can I have a quick pre-implementation review for this bug https://code.edge.launchpad.net/~adiroiban/launchpad/bug-49237516:34
abentleyadiroiban: looking16:35
abentleyadiroiban: I think it would be better to discuss this with someone from translations.  They have a clearer idea of the kind of changes they want in that area.16:36
adiroibanlib/lp/translations/browser/distroseries.py - I don't know if this is the right way to augment the view16:36
adiroibanabentley: thanks. no problem.16:37
=== adiroiban changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), noodles775 || queue [adiroiban(bug-492375 pre-implementation)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== beuno is now known as beuno-lunch
noodles775abentley: ok, back (sorry).16:59
noodles775So the behavior of raising an exception isn't something that has changed with this branch, I've just added a test for it (together with the fix for bug 475808).17:00
mupBug #475808: Unembargoing packages via the API doesn't seem to apply overrides correctly <ppa> <soyuz-security> <soyuz-upload> <Soyuz:Triaged by michael.nelson> <https://launchpad.net/bugs/475808>17:00
abentleynoodles775: Ah, I see.17:01
abentleynoodles775: So the override to main doesn't happen because the target isn't a PPA?17:04
noodles775abentley: yes, for ppas we always override to main, but not for other archives where we do support different components. We do a different override in that case (in SourcePackagePublishingHistory.overrideFromAncestry()), but that can only be done *when* we've created the SPPH.17:07
noodles775For delayed copies, this overrideFromAncestry() happens when the delayed copy is realised (see lp.soyuz.model.queue.PackageUpload.realiseUpload()) 17:08
abentleynoodles775: r=me17:09
noodles775Thanks abentley 17:10
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, - || reviewing: adiroiban(bug-435165), noodles775 || queue [adiroiban(bug-492375 pre-implementation)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
salgadohenninge, I have a RC one; can you take it?17:15
henningesalgado: I was about to leave. How big is it?17:16
salgadohenninge, https://code.edge.launchpad.net/~salgado/launchpad/bug-495544/+merge/1612017:16
salgadoreally small17:16
salgadohenninge, but it will be easy for me to find somebody else to review, if you'd prefer17:17
henningesalgado: no, that's ok17:17
henningesalgado: do you know about assertContentEqual?17:23
salgadoI once knew17:23
henninge;-)17:24
henningeit does the sorted() thing for you when comparing lists.17:24
salgadoyeah, I've even used it in the past, but this time I started with assertEquals as I had a test that was expecting an empty list.  and my test changed I forgot to use assertContentEqual17:27
salgadobut that's changed already17:27
henningesalgado: also, although I know it is not required, I like the (expected, observed) order better. If you could do that it would be nice.17:27
salgadosure thing17:28
henningesalgado: one last nitpick: TestZopelessTransactionManager_reset_store does not look right.17:32
salgadowhat would look right?17:32
henningesalgado: with the underscores in the name and having a Testcase to test just one method.17:33
henninge(that's what's not looking right)17:33
henningesalgado: Isn't it one test case per class and test_methodName_special_case in that class as a naming convention?17:34
henningesalgado: also, it looks like the other test case in that file is the special case as it has no layer17:34
salgadoI don't know about the one test case per class policy17:34
salgadotests with layers should be the exception, ideally17:35
henningeso how about calling the first one TestZopelessTransactionManagerNoLayer and yours just TestZopelessTransactionManager?17:35
salgadosure, that wfm17:35
henningecool, r=me17:35
salgadohenninge, thanks!17:36
=== beuno-lunch is now known as beuno
henningeadiroiban: I am sorry, I missed your ping earlier and saw it just now. Since your branch is not release-critical, I hope it's OK I finish the review later or tomorrow?`17:40
henningesalgado: oops, I just flicked the mp status back and forth, remembering that you still need an r-c review.17:41
henningejust so you don't wonder about the spam hitting your inbox ... ;)17:42
=== deryck is now known as deryck[lunch]
=== jelmer changed the topic of #launchpad-reviews to: on call: henninge, - || reviewing: adiroiban(bug-435165), noodles775 || queue [adiroiban(bug-492375 pre-implementation),jelmer]] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== deryck[lunch] is now known as deryck
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), jelmer || queue [adiroiban(bug-492375 pre-implementation),jelmer]] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), jelmer || queue [adiroiban(bug-492375 pre-implementation)]] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado is now known as salgado-afk
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [adiroiban(bug-492375 pre-implementation)]] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
jelmerabentley: Thanks!20:41
leonardrhenninge or abentley, will either of you have time today to review a short lazr.restfulclient branch?21:03
abentleyleonardr: I expect so.21:04
leonardrok, i'll write it up21:04
=== matsubara is now known as matsubara-afk
leonardrabentley, henninge: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/anonymous-credential/+merge/1616421:13
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: adiroiban(bug-435165), - || queue [adiroiban(bug-492375 pre-implementation), leonardr] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [adiroiban(bug-492375 pre-implementation)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
intellectronicaguess it was the irc connection...22:19
intellectronicaabentley: care to review a very small js fix which is a release blocker?22:20
abentleyintellectronica: I have EODed, but since it's a release blocker, go ahead.22:21
bacsinzui: got a sec to look at https://code.edge.launchpad.net/~bac/launchpad/bug-496701-mimetypes/+merge/16170 ?22:21
intellectronicaabentley: you rock. as you can imagine i've EODed quite a few hours ago :) https://code.edge.launchpad.net/~intellectronica/launchpad/manage-official-tags-js-error/+merge/1617122:21
abentleyintellectronica: r=me22:23
intellectronicaabentley: thanks a bunch22:24
intellectronicadanilos: can i land the fix above as rc?22:24
intellectronicaor flacoste?22:25
bacintellectronica: you should add them to the MP with 'release-critical' as the review type22:25
intellectronicabac: right. thanks for reminding me22:26
bacnp, intellectronica22:26
intellectronicabac: so is the drill now that i request a review from the release manager with release-critical as the review type?22:28
bacintellectronica: indeed22:29
bacintellectronica: there may be something else involving bug tags or marking the bug as critical, but i'm not sure22:29
intellectronicabac: yes, the bug is tagged 'release-blocker'22:30
intellectronicano, 'current-rollout-blocker'22:30
jmlhello reviewers22:42
jmlhttps://code.edge.launchpad.net/~jml/launchpad/fix-release-hot-bugs-486437/+merge/1617322:42
jmlnot urgent, of course, but maybe nice and simple22:42
bachi jml, could you put on your db-reviewer hat and look at https://code.edge.launchpad.net/~bac/launchpad/bug-490505/+merge/15691 ?22:47
baci didnt' think you'd be necessary on it but stub added you.22:47
jmlbac, sure, will do.22:48
jmlbac, is it RC?22:48
bacjml: good question.  we'd like to get it in this cycle but the review was delayed until yesterday.  so it isn't critical per se, but would be nice to have.22:49
jmlbac, ok.22:50
sinzuibac: am I correct in understanding that your branch change is to the python env that starts lp?22:50
bacsinzui: yes22:50
sinzuiyeah, that cryptic22:51
bacsinzui: that bit of voodoo is shown to work22:51
bacsinzui: i hate doing it there and hope we get a proper home for such stuff soon22:51
sinzuibac: I understand and agree. r=me22:52
bacthx22:52

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!