#launchpad-reviews 2010-04-05
<stub> https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680
<stub> https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680
<stub> jtv: ^^ that's a loop tuner patch if you are bored.
<jtv> stub: oooh, cool.  I'm working on an oversized review, but I may just take this one inbetween as a light snack.
* salgado changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* salgado changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<jtv> stub: had some lucid bugs, but looking at your branch now.  _is_timed_out should be spelled _isTimedOut; and "extra" is not a great name... maybe extra_seconds or extension_seconds or somesuch?
<jtv> (Though definitely nice to have that isolated)
<jtv> Maybe invert the condition and call it _haveTimeLeft(seconds=0)?
<jtv> ah no, that'd be the wrong way around wouldn't it
 * jtv continues to flip-flop on that issue
<stub> :)
<jtv> oh stub, there you are!  I just posted a comment.
<stub> ta. Just eating dinner :)
* abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup),salgado] || 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: abentley || reviewing: salgado || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<abentley> salgado, what proposal do you want me to review?
<salgado> abentley, remove-lp-service-openid
<abentley> salgado, lp:~salgado/launchpad/remove-lp-services-openid is not proposed for merging.
<salgado> I might have forgotten to create the mp
<salgado> yeah, looks like I did
<salgado> sorry abentley.  just proposed: https://code.edge.launchpad.net/~salgado/launchpad/remove-lp-services-openid/+merge/22807
<salgado> it depends on my remove-auth-store branch, which removes canonical-identity-provider
<abentley> salgado, cool.  I'll have a look in a few minutes when the diff is up.
<salgado> thanks abentley
<abentley> salgado, 1912 lines of diff!?
 * salgado checks
<salgado> abentley, the diff shown there includes what's in my remove-auth-store branch
<salgado> any idea why it didn't take the pre-requisite branch into account when generating the diff?
<abentley> salgado, no idea.
<abentley> salgado, are both branches up-to-date?
<salgado> yes, I think so.  let me try merging remove-auth-store again into the remove-lp-services one
<abentley> salgado, I believe you've got a criss-cross merge, because you merged devel into each of them.
<salgado> could be
<salgado> abentley, any way to solve that so that the generated diff is correct?
<abentley> salgado, if you merge remove-auth-store into remove-lp-services, it should be much better.
<salgado> abentley, just did that
<abentley> salgado, hmm.  So that brought it down to 943 lines.  Is correct now?
<salgado> very likely, let me check
<salgado> abentley, yes, that's correct.  it's just removals, though, as you can see
<abentley> salgado, this is usually when I ask about preimplementation calls, but I assume it's been discussed to death.
<abentley> salgado, r=me
<salgado> thanks abentley
* abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* sinzui changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<leonardr> abentley, looking for a quick review of https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header/+merge/22812
<abentley> leonardr, looking
<adiroiban> abentley: hi, is there anything I need to do for having this branch landed https://code.edge.launchpad.net/~adiroiban/launchpad/bug-548999/+merge/22271 ?
* abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: leonardr || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<abentley> leonardr, I believe there should be two blank lines, not one, between one heading and the next in xx-service.txt.  Otherwise, looks fine.
<abentley> adiroiban, it needs to be run through the full test suite.
<abentley> adiroiban, some other developers use ec2 to do this, but I don't use ec2.
<abentley> adiroiban, for my own branches, I use my local boxes.
<adiroiban> abentley: ok. I still have problems running the full test suite on my computer. Meanwhile, I will ask some other LP developer to send the branch to ec2. Thanks!
<abentley> adiroiban, no problem.
<abentley> sinzui, changing the topic does not get my attention, so I suggest asking me to perform the review instead of just changing the topic.
<sinzui> abentley, I agree. I was told to get my children from school so I did not complete my task
* abentley changed the topic of #launchpad-reviews to:  on call: abentley || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<abentley> sinzui, looking now.
<leonardr> abentley: actually i'm pretty sure you don't have to put 2 blank lines anymore, now that we changed the format from =foo= to foo\n===
<abentley> leonardr, the style guide uses "foo\n===" syntax, and says "Two blank lines are used to separate the start of a new section (a header). "
<leonardr> ok
<leonardr> abentley: i was going to write a launchpadlib branch with another test, and then i realized i can just put a launchpadlib test in launchpad now
<leonardr> so i'll give you an incremental diff in a bit
<abentley> sinzui, I think that "an unlisted package" may not convey "not packaged".
<sinzui> abentley, I should hope NOT
<sinzui> abentley, it is not related to the second button. It means the user has to find a package himself using +ubuntupkg
 * sinzui think the UI review will be equally scathing
 * sinzui needs to think how to say ":let me pick the ubuntu package myself"
<abentley> sinzui, I see.  This is the page that is affected by the "not packaged" indicator, not the page where users select that?
<sinzui> The not packages button does not use the radio button. It is a simple button that means stop showing me the form
<sinzui> abentley, the problem is that this simple button will not fit in the the portlet. so we discussed moving the link to +ubuntupkg into the vocabulary. That was easy, but the intent is clearly ambiguous now.
<abentley> sinzui, I am confused about why you are showing me a version with and without a selection.
<abentley> s/selection/suggestion
<abentley> sinzui, would it reasonable to present "not packaged in ubuntu" as a radio button?
<abentley> sinzui, in lib/lp/registry/interfaces/product.py, I think you mean "project", not "projected", in "The projected can be rechecked after enough time "
<abentley> sinzui, I don't know what to do about this, but it seems odd that if I select "This is not packaged in Ubuntu", in a year it will prompt me to specify what package it provides in Jaunty.
<abentley> sinzui, ping me when you want to proceed.
* abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<bac> sinzui: could you give a UI once over to https://code.edge.launchpad.net/~bac/launchpad/bug-524302/+merge/22180
<bac> sinzui: i just pasted a screenshot into the last comment
 * sinzui is already doing that
<sidnei> i have a large branch needing review here, but im on holiday today. if anyone wants to take a peek, feel free, but no rush: https://code.edge.launchpad.net/~sidnei/lazr-js/yui-3.1.0/+merge/22825
<james_w> abentley: thanks for the review. https://code.edge.launchpad.net/~james-w/launchpad/register-code-import/+merge/22668 is updated if you could have another look.
<rockstar> james_w, I suspect abentley has EOD'd for the day.  :)
<james_w> rockstar: me too, but I thought it was worth a try as the topic hadn't changed
<james_w> but it's not important given that I'm not really here
<rockstar> sidnei, are you around?
<sidnei> rockstar, just happen to be
<rockstar> sidnei, so, I'm confused a bit by your branch.  Why are we renaming classes, and what are all the other changes as well?
<rockstar> It LOOKS like it's YUI 3.1 stuff.
<sidnei> rockstar, yui 3.1.0 changed the prefix generated by ClassNameManager from yui- to yui3-
<rockstar> sidnei, ah, okay.  So is this the branch that will make lazr-js support yui 3.1?
<sidnei> rockstar, correct
<sidnei> rockstar, the other changes are two kinds: 1) removing wait/resume where tests were failing, seems like it is not needed anymore in those cases and 2) double checking for null nodes
<rockstar> sidnei, okay.
<rockstar> sidnei, that helps.  I'll look over this in a bit, and you should have a review for you in the morning.
<sidnei> rockstar, awesome. thanks!
<sidnei> rockstar, if you get a chance, try the examples too. i noticed one or two misbehaving but didn't dig much yet. probably trivial to fix. the tests pass though.
<rockstar> sidnei, okay.  Well, if they don't work, we probably need to figure out why before we land this.
<sidnei> rockstar, one of them was the inline editor, when clicking on edit the static text isn't hidden. likely a css issue.
#launchpad-reviews 2010-04-06
* thumper changed the topic of #launchpad-reviews to: on call:  || reviewing: - || queue: [sinzui] || 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: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<stub> henninge: https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/22849
<stub> henninge: Security issue that needs to land before the edge update (about 55 mins?)
<wgrant> stub: The branch isn't in stable yet.
<wgrant> it's fine.
<henninge> stub: looking
<wgrant> (and probably won't be until some hours after edge updates tonight)
<henninge> stub: Shouldn't that comment be an XXX that links to a bug number where the issue is tracked?
<stub> Its not a bug though.
<stub> Actually, I just realized I need to turn off caching entirely to not lose functionality, as the tooltips will continue to display old summaries when bugs are changed.
<stub> I could put a lower timeout, but that is getting really pointless.
<henninge> stub: unfortunately I am not at all familiar with caching issues ...
<henninge> stub: but this still sounds like a quick fix to me that needs further investigation to find a proper solution and we should track that in a bug in order not to forget it.
<stub> I'm not sure what the bug is. Marked up test is uncachable due to Launchpad features. We either put up with it, or change Launchpad features. Its a discussion.
<stub> I've removed the cache: stanza entirely, so the diff is just a reversal of some work that landed an hour ago and an additional comment. I can remove the comment if you like, as I think it is throughout Launchpad rather than a bug comment specific issue.
<henninge> stub: ok, that sounds reasonable.
<stub> henninge: Reasonable enough for an approve vote?
<henninge> stub: certainly, I was waiting for the new push ... ;-)
<henninge> stub: r=me
<stub> Ta :)
* henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<jml> henninge, could I persuade you to take a look at cjwatson's branch?
<henninge> jml: branch of what? ;-) I don't see anything on activereviews...
 * jml looks
<jml> ahh, he asked deryck specifically
<jml> never mind
<deryck> jml, henninge -- sorry, I missed he requested the review due to mail filters.  Will get it today.
<jml> deryck, thanks.
* henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [] || 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: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* bac changed the topic of #launchpad-reviews to: on call: henninge, bac || reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<bac> good morning
<bac> hi deryck
<deryck> bac, Hi.
<bac> deryck: did you coordinate with brian at all on https://code.edge.launchpad.net/~brian-murray/launchpad/fix-date-closed-definition/+merge/22836?
<bac> it's really trivial but appears to not have had a pre-imp call (required but overkill) nor manage the bug at all
<deryck> bac, not having looked yet, I don't recall talking about that with him, no.  looking now....
<bac> deryck: just thought you may want to speak to him about triaging the bug and claiming it before starting work
<bac> the fix itself is trivial...
<deryck> bac, sure, we can discuss it.  He's on my team next month, so it becomes even more important then. :)
<bac> great
<deryck> bac, I suspect he does sooooo much bug triage currently that he just wanted to drive by fix something and not worry about the rest. :-)
<bac> walking on the wild side, he is
<deryck> ah, I had never triaged the bug initially either.  He was likely waiting on me.
<deryck> all fixed up now.
<wgrant> bac: Sorry, I was planning to get Soyuz approval, but we have been rather lacking in the Soyuz department for the past several days.
<bac> wgrant: yeah, with julian out of pocket i can see that
* henninge changed the topic of #launchpad-reviews to: on call: henninge, bac || reviewing: - || queue: [] || 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, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* james_w changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, - || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<james_w> henninge, bac: https://code.edge.launchpad.net/~james-w/launchpad/code-import-request/+merge/22727 when you get a few minutes please
<bac> james_w: will do
<james_w> Merci
<sinzui> bac: can you look at http://people.canonical.com/~curtis/package-suggestion-portlet.png and think about how to fix the UI? I am tempted to revert the a button and link for packaging links, and moving the Not Packaged button somewhere else in the portlet
<james_w> sinzui: heh, looks like you've already fixed the bug I just filed :-)
<leonardr> abentley, can you take another look at https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header/+merge/22812 ?
<abentley> leonardr, sure.
<leonardr> gary, i'd like your opinion on the Vary:Authorization issue as well
<leonardr> abentley, give it a minute to regen the diff
<sinzui> james_w, which bug?
<james_w> sinzui: filed like 2 minutes ago, that "[ Link to this Ubuntu package] or (+) Link to Ubuntu Package" confused me
<gary_poster> leonardr: ack will look
<sinzui> oh, damn, this UI is going to kill me since the current one really confused abentley
<sinzui> bac: james_w maybe we need a list of options with a single button. Two of the options are "another unlisted package" and "this project is not packaged in Ubuntu"
<james_w> sinzui: that would work for me. I'm also ok with your mockups there.
<abentley> sinzui, what happened to you yesterday?  You disappeared while I was asking review questions.
<sinzui> henninge, you could not see anything in devel because sample data was wacky. It will be fixed soon. That is why I create a screen shot...you do not want to see my mark harness changes to create sane spph
<sinzui> abentley, you assumed you needed to choose "an unlisted package" to use the "This is Not Packaged in Ubuntu" button.
<abentley> sinzui, I assumed that was meant to be the constant item between the two screenshots.
<abentley> sinzui, I asked you a bunch of questions yesterday that you did not reply to.  One was "would it reasonable to present "not packaged in ubuntu" as a radio button?"
<sinzui> It is. the option means "select another package not in this list" and maybe that is the wording I should have used
<gary_poster> leonardr: xx-service.txt looks like it would fail now.  Your code says self.response.setHeader('Vary', 'Accept') but your test says
<gary_poster> 83	+    >>> response = webservice.get(
<gary_poster> 84	+    ...     'http://bugs.launchpad.dev/api/devel/')
<gary_poster> 85	+    >>> print response.getheader('Vary')
<gary_poster> 86	+    Authorization, Accept
<leonardr> gary: thanks, don't know how i missed that
<sinzui> abentley, I think that is the best solution. I need to find new wording for the button.
<abentley> sinzui, Yeah, but the submit buttons were also constant.
<gary_poster> cool
<leonardr> maybe i spent all my time writing that comment and didn't rerun the test
<bac> sinzui: i don't think "an unlisted package" makes sense, especially in the "portlet without suggestion" example
<sinzui> bac: "select a package using the package finder?"
<bac> yes, better
<bac> perhaps when there are suggestions "select some other package using the package finder" and what you wrote for the 'without suggestions' case
* bac changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, wgrant || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* sinzui changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, wgrant || queue: [james_w, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<sinzui> bac: henninge: if you have time, can you review https://code.launchpad.net/~sinzui/launchpad/moderate-empty-messages/+merge/22870
<henninge> bac: you had agreed to do james' branch, right?
<bac> henninge: no, i was speaking for the both of us.  if you are free it is next up.
<henninge> bac: ok, I'll take that first, then.
<bac> thanks henninge.  how do you like OCR on tuesdays?  much better, right?
<henninge> bac: yeah, a lot better ... ;-)
<henninge> although, to me this Tuesday is like a Monday ... ;)
<bac> ah, right, ye of the four day weekend...
<henninge> ;-)
<henninge> over much too quick ...
* henninge changed the topic of #launchpad-reviews to: henninge, bac || reviewing: james_w, wgrant || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<henninge> james_w: did you have a pre-implementation chat/call with someone from the code team about requestImport? (more recent than the Karmic UDS)
<james_w> I did not
<henninge> james_w: Especially when adding stuff to the API, I think it's better to be sure that you and them are still on the same page.
<henninge> james_w: So, maybe simplest would be to ask somebody from that team to review the branch, then. OK?
<james_w> sure
<henninge> thanks
* henninge changed the topic of #launchpad-reviews to: henninge, bac || reviewing: sinzui, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<abentley> leonardr, I don't know if you're still wanting my re-review.  I was waiting for the test fix.
<leonardr> abentley: should be pushed, let me check
<abentley> leonardr, maybe my page was stale.
<leonardr> abentley: yes, it's rev 10623
<abentley> leonardr, looks fine.  r=me
<leonardr> great
* henninge changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<henninge> sinzui: review done.
* bac changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
 * bac high-fives henning...empty queue
<henninge> yeah, cool! ;)
 * sidnei wonders where rockstar's review stands
<rockstar> sidnei, it's a big review.  :)  I'm working on it though.
<sidnei> cool, thanks!
<rockstar> sidnei, so, maybe I should ask you: could you please write up a doc or email or something detailed the changes necessary for yui 3.1?
<sidnei> rockstar, i don't know all the changes necessary yet. so far only lazr-js specific, and those were mostly css and making the tests pass, which only involved changing tests themselves.
<rockstar> sidnei, okay.  Could we you maybe start a wiki page on the dev.launchpad.net about it then, so we can add to it?
<sidnei> rockstar, sounds like a plan.
<rockstar> sidnei, basically, I want a doc that shows us a clear migration plan of the things that definitely need to change.
* henninge changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<sidnei> rockstar, https://wiki.canonical.com/Rhinos/YUI/MigrationIssues
<rockstar> sidnei, great.  Can you send that to the Rhinos list?
<sidnei> rockstar, sure.
<rockstar> bac, can I hop in your queue in about 15?
<bac> rockstar: sure
<rockstar> bac, it'll be a bit.  Dealing with conflict hell currently.
<rockstar> bac, so, here's the little one: https://code.edge.launchpad.net/~rockstar/launchpad/branch-bug-display/+merge/22887
<rockstar> bac, the other one I'm still trying to sort out dependent branch on.
<bac> ok
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [rockstar1, rockstar2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<leonardr> gary, ec2 turned up some very minor test failures on https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header . can you review my recent changes?
<leonardr> i also had to do a launchpadlib branch, which is here: https://code.edge.launchpad.net/~leonardr/launchpadlib/test-fix/+merge/22905
<leonardr> abentely, if you're still around, you could take care of this instead
<leonardr> abentley
<salgado> bac, care to review https://code.launchpad.net/~salgado/launchpad/bug-556594/+merge/22906 for me?
<bac> salgado: sure
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: salgado || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<abentley> leonardr, r=me
<leonardr> great
<gary_poster> leonardr: on call. do you still need me?
<leonardr> gary: no
<gary_poster> ok cool
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<rockstar> bac, here's the other one to review: https://code.edge.launchpad.net/~rockstar/launchpad/ihasrecipes/+merge/22890
<salgado> thanks for the review, brad.  (and I did that change you suggested)
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: rockstar || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<EdwinGrubbs> bac: I have a tiny branch that will be ready in a couple of minutes.
<bac> EdwinGrubbs: ok
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<salgado> bac, if you have a few more minutes, I've updated that m-p you just reviewed with the actual fix for the bug
<bac> salgado: ok
<EdwinGrubbs> bac: here it is: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-552619-suggesting-removed-packages/+merge/22912
<bac> EdwinGrubbs: i'm way ahead of you.  :)
* bac changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
#launchpad-reviews 2010-04-07
<gary_poster> I need a testfix review
<gary_poster> http://pastebin.ubuntu.com/410297/
<gary_poster> The problem was that buildbot is the only thing that runs ``python2.5 -t test_on_merge.py -vv``
<gary_poster> ec2 test does it differently (not sure how) so it was missed.
<gary_poster> I'd prefer to change buildbot to simply run ``test_on_merge.py -vv`` at which point I can remove the nasty os.execv
<gary_poster> but this lets us move on for now
<gary_poster> mwhudson: ping?
<mwhudson> gary_poster: looking
<mwhudson> gary_poster: that's horrible
<gary_poster> yes it is
<mwhudson> gary_poster: changing buildbot isn't that hard, really
<gary_poster> ...no, but I should be giving my son a bath :-)
<mwhudson> at least if there's a losa around
<gary_poster> I suppose I could just back my branch out
<mwhudson> gary_poster: how should buildbot invoke test_on_merge.py ?
<mwhudson> gary_poster: just with ./bin/py ?
<gary_poster> mwhudson: that would work.  Invoking it with ``test_on_merge.py -vv`` would also work.  Then the diff woud be this:
<mwhudson> gary_poster: like so https://pastebin.canonical.com/30183/ ?
<gary_poster> mwhudson: exackle.
<mwhudson> gary_poster: i can get that change landed if you like
<mwhudson> it seems better than that pastebin, tbh
<gary_poster> mwhudson: testfix would then be http://pastebin.ubuntu.com/410305/
<mwhudson> gary_poster: ah right
<gary_poster> mwhudson: I'll see if a losa is around.  If so, will do it.  If not, will take you up on your offer with gratitude
<mwhudson> gary_poster: want to review https://code.edge.launchpad.net/~mwhudson/lpbuildbot/bin-py-test/+merge/22918 ?
<mwhudson> it's probably a good idea in any case
<gary_poster> reviewing
 * mwhudson wonders if anyone wants to review https://code.edge.launchpad.net/~mwhudson/launchpad/allow-import-password/+merge/22924
<jtv> mwhudson: I'm on it
* StevenK changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<jtv> mwhudson: there's no chance that these passwordful URLs will show up in the UI?
<mwhudson> jtv: there's every chance
<jtv> mwhudson: isn't that, like, potentially bad?
<mwhudson> jtv: not really, it's for cases that require guest:guest as the username/password combo
<mwhudson> jtv: i guess some words in the new code import form might be a good idea
<jtv> mwhudson: the blink tag almost seems like a good idea here, if you get my point...
 * wgrant banishes jtv.
<jtv> hey, I said "almost"!
<mwhudson> jtv: in general, the result of the import is going to be publicly available
<jtv> mwhudson: that's fine, but one thing that gives a black-hat an even better guess to all of the average user's passwords than their birthday is... one of their passwords.
<jtv> Not that bzr users are likely to be average computer users.  But OTOH they're likely to manage more accounts than most.
<jtv> Another way to do it would be to obscure the password when displaying the URL.
<jtv> mwhudson: Actually I'd prefer the latter because it also protects people who enter a login that has commit rights.  Whether those people deserve protection is a matter for BOFHs and PFYs, I guess.  :-)
<jtv> mwhudson: I have to relocate now.  I've added my comment to the MP.
* jtv changed the topic of #launchpad-reviews to: On call: jtv || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<jtv> On call: jtv || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<StevenK> jtv: Hi!
<jtv> hi StevenK
<jtv> Your code looks good, but the MP is a bit thin!  Pre-imp?  Lint?
<StevenK> jtv: Uh? I'm new here, can you expand 'pre-imp' ?
<jtv> Ah.  "Pre-implementation call."
<jtv> We're expected to discuss each change with someoneâwhether it's someone more knowledgeable about the topic, or a peer, or an outsider, or someone less experiencedâand make every effort to make that discussion by voice.
 * wgrant imagines the content of the call as "AAAAAAAAAAA it's broken it's broken fix it fix it"
<jtv> wgrant: the intended solution should come up at least in passing, thank you.
<StevenK> jtv: Oh, right. In both MPs I've got up for review, I've spoken to both Julian Edwards and Michael Nelson via Skype about them
<jtv> But otherwise, wgrant hit it on the nose.  :)
<jtv> Good!
<jtv> Because as a non-Soyuz person I'm hardly in a position to know whether this change is as sane as it looks.
<jtv> And with "lint," I mean the output of "make lint."  I'm not assuming you didn't run it, but we normally at least mention it in the MP as a reminder.
<jtv> My own branches generally have a final commit around MP time with the description "Lint."  Shows how effective that procedure can be.
<StevenK> I've been reading through the coding standards, but I have yet to get into the habit of running make lint before submitting the MP
<jtv> Then this would be a good start!
<jtv> Good *time to* start, that is
<StevenK> Well, running it just blew up with a wierd error, so \o/
<jtv> Unfortunately a lot of our standards aren't properly documented afaik
<jtv> uh-oh
<StevenK> OSError: [Errno 17] File exists: '/home/steven/launchpad/lp-branches/fixes-bug-387049/eggs/setuptools-0.6c9-py2.5.egg'
<jtv> Always fun how EEXISTS never mentions why this should be a problem.
<jtv> You could try removing it, but then you'd probably also have to re-run rocketfuel-get; make clean; utilities/link-external-sourcecode ../devel; make
<jtv> (And possibly merge devel somewhere in that sequence as well)
 * jtv tries the branch out locally
<StevenK> Well, I'm happy to make clean and then link and make lint
<jtv> I can do it hereâyou ran the test though, right?
<StevenK> The test suite? Yes
<jtv> So it's not really worth going through the pain just for the lint.  Hang on, I'm setting things up on my end.
<jtv> About the code itself: nice to know about the test publisher's addMockFile and addFakeChroots; I wasn't aware of those.
<jtv> StevenK: ah, this should be helpful: https://dev.launchpad.net/CoverLetters
<jtv> StevenK: meanwhile, I have a call coming up.  That means I'll drop off for a while, without much warning.
<StevenK> Right, I have make lint working and running at least
<jtv> oh good
<StevenK> But understanding the output is the next problem
* allenap changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: -, - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<allenap> Morning jtv-otp :)
<jtv-otp> hallo allenap
<jtv> StevenK: the core of the change could be made a tad easier to read, I think (always a concern with these long soyuz identifiers).
<jtv> StevenK: something like
<jtv> dest_file = dfc.get(file.libraryfile.filename)
<jtv> if dest_file is not None and file.libraryfile != dest_file:
<jtv> (Saves an indent level as well)
<jtv> StevenK: _but_ "file" is not a good variable name since it's also a built-in class.  This has bitten me many times, though not usually very hard.
<StevenK> Yes, it has me too. I'm not sure why I keep doing 'for file in ...'
<jtv> If it bit harder, I'm sure we'd all have learned by now.  :-)
<StevenK> jtv: So I've got the output from make lint, with stuff like: 652: [E1002, CopyCheckerTestCase.setUp] Use super on an old style class
<jtv> StevenK: that particular message is...  "a bit confused" afaict.  You have my wholehearted fiat to ignore that one until we know what python version we're really at.
<StevenK> Hehe, right
<StevenK> jtv: So, I'll clean up that code tomorrow and push a branch
<jtv> My compliments on your commenting habits: it's no more and no less than what's needed.  The one at the top of the diff though is part of a "# Do X" pattern.  It would probably be nicer to have the new check in a separate method, and its name could do all the explaining needed.
<StevenK> jtv: I thought about doing that -- but the method is already doing lots of checks, and it was either adding one more, or a refactor
<jtv> Well one has to start somewhere...  I'm not particularly keen on fixing this pattern, usually, but when you build up real data structures, especially in longer methods, it's often very helpful to minimize their scope.
<jtv> I do agree that it's ugly to have different levels of abstraction in the method; maybe it's something that fits better one level up in the call tree?
<StevenK> jtv: Thinking about it more, this method already has a large cache structure that is used by a large number of the checks, and passing that around just makes me feel even uneasier
<jtv> StevenK: is this an urgent fix, something you may want to CP?
<StevenK> jtv: It isn't, no
<StevenK> jtv: It's a nice to have, since it may bite people who use the package copier
<jtv> Because when you hit this sort of dilemma, it's probably best to "bzr shelve" your changes (or create a pipeline) and put up a separate refactoring branch first.  Of course by the time one realizes this, it's often water under the bridge.
<StevenK> Heh, yes
<jtv> Looking at the diff, ISTM you only need to pass a few things if you extract this (into a standalone function afaic): destination_file_conflicts and source.sourcepackagerelease.files.
<StevenK> jtv: Right, what I was getting at, was if I abstract this bit out, why not the rest too?
<StevenK> Where my concern about copying around large structures came from
<jtv> Well, that would be good too, definitely!  I thought you were concerned that I might force you to, and was trying to assuage that concern.
* jtv changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: StevenK, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<StevenK> Wah, now bin/test is blowing up
<wgrant> I have a severely oversized (~9500 lines) branch, but it is exclusively removals. Can I can convince somebody to take it?
<jtv> wgrant: imagine a very noncommittal voice here... removals of what?
<wgrant> jtv: Lots of unused code. A JS library or two, an extraneous JS minifier, some directories named 'not-used', a couple of Makefiles that have been unused and broken for three or four years...
<StevenK> jtv: So I have the refactoring changes, but bin/test is blowing up with ImportError: No module named gettextpo
<jtv> wgrant: wow...  for the JS stuff at least, I'd ask sinzui; he probably has the best overview of what can go.
<jtv> StevenK: you may need to "make" again
<StevenK> Ahh
<jtv> StevenK: the only lint I see btw is test_copypackage.py importing TestCase but not using it.
<StevenK> jtv: Right, which I've fixed locally
<StevenK> Whee, Failed to load application: No module named psycopg2
<wgrant> StevenK: Your version of python-support or python-psycopg2 is wrong.
<wgrant> Ah, no, just the latter.
<StevenK> So karmic's version of python-psycopg2 is wrong?
<wgrant> You need one from the Launchpad PPA.
<StevenK> Which is installed
<StevenK>  *** 2.0.8-0ubuntu3~launchpad1 0
<wgrant> What happens if you try to import psycopg2 from a non-buildout-tainted python2.5?
<StevenK> wgrant: It works
<wgrant> <3 buildout <3
<StevenK> Hah, nice. :-(
<allenap> StevenK: You have two proposals needing review. Shall I take one?
<StevenK> allenap: fixes-bug-553068 is the other, so sure
<allenap> StevenK: Cool.
* allenap changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: StevenK, StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* noodles775 changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: StevenK, StevenK || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<noodles775> Hi jtv, allenap. This one's already been reviewed, but I had to create a new MP after failing to merge a new production-devel: https://code.edge.launchpad.net/~michael.nelson/launchpad/pd-550049-overwriting-buildd-manager-files/+merge/22932
<jtv> noodles775: yowza... "you don't have permission to access this page"
 * noodles775 adds jtv
<noodles775> jtv: try now.
<jtv> yup, that works.
<noodles775> Thanks.
<jtv> noodles775: two very different ideas on where buildid should come from, there...
<jtv> Used to come from the slave, now comes from the master.
<jtv> What method is that in?  I'm talking about lines 17â18 of the diff.
<noodles775> jtv: from the build itself. If you look at the original MP, there's a note there about it.
<noodles775> jtv: and it's _handleStatus_OK
 * wgrant points at the channel privacy and coughs a bit.
<jtv> wgrant: thank you, it had percolated through in the meantime
 * noodles775 moves the discussion elsewhere :)
<wgrant> Thanks.
<jtv> StevenK: any progress on the 387049 branch?
<StevenK> jtv: You know it's 10pm here, right? :-)
<jtv> StevenK: no, but thanks for telling me.  :)
* jtv changed the topic of #launchpad-reviews to: On call: allenap || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<jtv> eod here too...
<gmb> allenap, Could you review the code portion of https://code.edge.launchpad.net/~gmb/launchpad/not-nullify-bwa.result-bug-546774/+merge/22934 for me?
<gmb> stub, BjornT, I've requested DB reviews from you guys for https://code.edge.launchpad.net/~gmb/launchpad/not-nullify-bwa.result-bug-546774/+merge/22934, too.
<gmb> stub, Thanks for the review :)
<allenap> gmb: Sure.
* allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: gmb || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<jtv> allenap: hadn't even noticed you were reviewing my branch... thanks!
<allenap> jtv: Cool, you're welcome :)
<jtv> :)
<jtv> and with that, jtv eods
<jtv> good night!
* sinzui changed the topic of #launchpad-reviews to: On call: allenap || reviewing: gmb || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<sinzui> allenap, EdwinGrubbs: can you review https://code.launchpad.net/~sinzui/launchpad/newline-removal/+merge/22915
<EdwinGrubbs> sinzui: sure
<gary_poster> kfogel: re https://code.edge.launchpad.net/~kfogel/launchpad/google-analytics-everywhere/+merge/22891 , it's fine.  I would remove the lines with only ellipses, because I don't think they buy anything, but if you disagree, I'm ok with it as is.
<kfogel> gary_poster: that's a good idea; I wasn't sure quite how "..." matching worked and actually meant to test without those lines, then forgot to.
<gary_poster> kfogel: it's very greedy, think ``.*``
<kfogel> gary_poster: ok
<gary_poster> kfogel: fwiw actually, I think ``.+`` is more accurate
<sinzui> BjornT: can you look at the proposed schema/interfaces changes in https://code.edge.launchpad.net/~sinzui/launchpad/not-packaged/+merge/22810
<kfogel> gary_poster: :-)  gotcha
<BjornT> sinzui: sure
<gary_poster> kfogel: approved
<kfogel> gary_poster: thanks.  (I committed the change you suggested; test still passes)
<gary_poster> :-) cool
<BjornT> sinzui: i have some issues with the name. also, only using a date seems a bit unclear. did you consider other options?
<sinzui> BjornT, We considered recording the series, but that leads to asking redundant questions...
<sinzui> The new series is always identical to the series just released...not enough time has passed for the series to diverge...
<sinzui> So we think 1 year is about the right time for users. the real time is closers to 9 months
<sinzui> BjornT, If someone can suggest a better name for the date field I will happily change it.
 * sinzui struggled with the name
<BjornT> sinzui: did you consider using the existing Packaging table for this?
<sinzui> BjornT, only briefly. The schema requires a distroseries. I think it would get message to create a entry without a distroseries
<sinzui> BjornT, and it requires a SPN, which the project does not have.
<sinzui> s/message/messy/
<BjornT> sinzui: well, the db schema doesn't require it, as far as i can see, so it's easily changable.
<sinzui> what?
<sinzui> The disrtoseries+spn is a unique key I think
<BjornT> sinzui: they can still both be null.
<sinzui> BjornT, you propose that we create a false packaging link for the project. And we modify product+packages to hide the false packaging link.
<BjornT> sinzui: i want you to consider using the Packaging table, since it gives you the date and who said it wasn't packaged for free.
<sinzui> right
<BjornT> sinzui: i'm not proposing a false link. i propose to say that it isn't linked to a source package
* allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<sinzui> We do not really delete links. So if I have a false link, and then visit +ubuntupkg, do I need to delete the false link when I create the new one
<sinzui> BjornT, I suppose so, and I need to delete it for sp+linkpackage
<BjornT> sinzui: yes, or update it. we do allow links to be deleted, now?
<BjornT> sinzui: in short, you should do the same as you do when links are edited.
<sinzui> We cannot update it in the latter case, Remember that from the sp's perspective there is a 1-1 relationship and it queried for iteself and found None. projectseries know the relationship is *-*
<sinzui> BjornT, yes, we allow users to delete links. the application creates them, or updates the distroseries-spn version. This is a minor detail. I'll give the Packaging approach a try
<BjornT> sinzui: well, you can always retrieve the None-link, if you have no link. it's the same things as if you want to link a package to a project that already has a link to another package, except you don't have to ask the user what the right link should be.
<BjornT> sinzui: cool. whether to use the distroseries is an open question. setting it to the current series might be good, but i'm not sure. i suspect it's more correct to set it, since Packaging (at least in theory) is used for more than Ubuntu. (or isn't it allowed to link to non-Ubuntu packages?)
<sinzui> We removed the feature to allow projects to link to other distros. Other distros can link to a project, but given the fact we only have spph for Debian, only Debian can get a link
<sinzui> distroseries + spn is unique. I really really do not want to change that rule. This rule as added to prevent the insane data we collected for the first 3 years of the feature
<sinzui> BjornT, As I think about the distroseries+spn constraint, I do not think Packaging can be used. We need to manufacture an spn for each project
<BjornT> sinzui: there shouldn't be a need to manufacture an spn. the current constraints allow spn to be null.
<BjornT> sinzui: you might have to to add a constraint to make sure there's only one null spn for a project/distroseries, though
<sinzui> I do not think we can have (null, null, gdp/trunk) and (null, null, launchpad/trunk)
<BjornT> sinzui: you can with the current constraints
<sinzui> Bjorn: None isn't acceptable as a value for Packaging.distroseries
<sinzui> BjornT, storm does not like it
<sinzui> BjornT, We need to change the model to allow spn and distroseries to allow None
<BjornT> sinzui: well, that's because storm has been told it can't None, even though the db schema allows it
<BjornT> sinzui: yes, you need to update the model class and interface for this to work. you probably won't come up with a solution that doesn't require any model changes :)
<sinzui> Deleting a product series and the latest packages get complicated. We will need to revise or add methods that get SPs to know that they can get meaningless results that need filtering
<BjornT> sinzui: i don't see what would be complicated. could you expand?
<sinzui> Product.sourcepackages must change to ensure false packaging links are not uses...conversely, this is the method we use when choosing to ask the user to link to  package. We need another method link sourcepackages that does not filter
<BjornT> sinzui: right. i wouldn't call it complicated, though, just normal code changes, which you generally have to deal with :)
<sinzui> BjornT, The same is true for productseries. And setPackaging can be taught to work with false packages.
 * BjornT -> phone
<sinzui> BjornT, okay, I will try. this will be much more invasive, requiring many more code changes to disambiguate the Packaging object.
<leonardr> allenap, for the queue: https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-service-root/+merge/22948
<allenap> leonardr: I'll do it now.
<leonardr> great
<allenap> leonardr: Oops, I didn't see sinzui's entry. sinzui, is that the newline-removal branch?
<sinzui> allenap, yes it is
<allenap> sinzui: Okay. leonardr, I'll do yours after sinzui.
* allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: sinzui || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<allenap> leonardr: Is there any way config.active_versions could be empty?
<leonardr> allenap: no, that would prevent lazr.restful from starting up
<allenap> leonardr: Instead of having [604800, 3600], could you have a module-level constant like `HOUR = 3600 # seconds`, then the default can be specified as [7 * 24 * HOUR, 1 * HOUR]?
<leonardr> sure
<EdwinGrubbs> allenap: oops, I forgot to take sinzui's name out of the queue and claim the review. I didn't mean to re-review it.
* EdwinGrubbs changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<sinzui> I have 3 reviews? I must be special
* allenap changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<allenap> EdwinGrubbs: All's well; you spotted a couple of things I missed.
<leonardr> allenap, i had to make one more minor change to https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-service-root/+merge/22948
<allenap> leonardr: I'll take a look.
<allenap> leonardr: Looks good.
<allenap> EdwinGrubbs: If you have a moment, please could you sanity check the diff I've attached to https://code.edge.launchpad.net/~allenap/launchpad/dynamic-batch-size-bug-546085/+merge/22538
<allenap> EdwinGrubbs: I added a constant to signify an unlimited size batch, BATCH_SIZE_UNLIMITED, but it's 0, not object(). However, the meaning when batch_size is None is now unambiguous: it always means unset, never unlimited.
<leonardr> allenap, looks like i need to do still more work on the lazr.restful side, i'll ping you when it's ready
<allenap> leonardr: Okay, no worries. I'll be afk in ~20 minutes, but I'll look at it in the morning.
<leonardr> allenap: i may have someone else look at the updates
<leonardr> in that case
<allenap> leonardr: Okay.
<EdwinGrubbs> allenap: I'm surprised that you added a line to tests/externalbugtracker.py to set the batch_size to BATCH_SIZE_UNLIMITED, but it didn't change the results of any of the tests.
<allenap> EdwinGrubbs: It kind of does the same as lines 153 to 157 used to do.
<allenap> EdwinGrubbs: ... because most calls into updateBugWatches() and _getRemoteIdsToCheck() didn't specify batch_size.
<EdwinGrubbs> allenap: ok, that makes sense. Looks good.
<allenap> EdwinGrubbs: Thanks :)
<EdwinGrubbs> mars: I see that you have two merge proposals for the same branch to merge it into devel and db-devel. Can one of those be deleted?
* EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
#launchpad-reviews 2010-04-08
<wgrant> abentley, bac: Can you please land my branches that you reviewed? They're all Soyuz-approvedâ¢.
<abentley> wgrant, I'm not currently able to run the test suite to completion.
<wgrant> abentley: Urgh. Anyway, mwhudson has EC2ed them.
<abentley> mwhudson, thanks!
<abentley> wgrant, wait-- do you mean that they've been tested but mwhudson isn't landing them.
<abentley> ?
<wgrant> abentley: They are in the process of landing through ec2.
<abentley> wgrant, cool.
<thumper> https://code.edge.launchpad.net/~thumper/launchpad/package-branch-listing-speedup/+merge/22993 1070 lines
<thumper> sorry
 * thumper popping out to get real coffee and drop off some children's dvds back to the video store
<StevenK> thumper: The question is, who watched them?
<thumper> StevenK: I watched a part of the black stallion, but not all, nor the sequal
<wgrant> Is it just me, or does download-cache only have a 2.6 x86_64 meliae egg, and not a 2.6 i386 one?
<StevenK> Sigh, devkit-disks, I hate you.
<StevenK> Why does it only mount Mass Storage devices *sometimes*
<wgrant> StevenK: Isn't that one of DX's new decisions?
<StevenK> Oooh, bitter much
<wgrant> Well, "why not" is enough rationale.
 * StevenK puts his fingers in his ears and says Lalalalala a lot
<wgrant> Heh.
<jtv> mwhudson: hi, any news on allow-import-password?
<mwhudson> jtv: no, got kind of sidetracked
* noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<noodles775> jelmer: do you want to take a look at james_w' branch first? (it's a non-soyuz one ;) ), otherwise I'll jump right in.
<james_w> it's rather simple :-)
<noodles775> Oh, I was going by the line count before you resolved the conflicts :)
<noodles775> jelmer: nm.
<noodles775> lol
<noodles775> r=me
<james_w> thanks
<james_w> please twiddle the knobs and I'll throw it at ec2
<noodles775> Done.
<james_w> and I need to use [testfix], correct?
<noodles775> Yep, `bzr log --limit 50 | grep testfix` will show you an eg or two.
<james_w> thanks
<james_w> sent
 * james_w just used --textfix
 * noodles775 didn't know about that option... great.
<bac> wgrant: is mwhudson landing all of your approved branches or do you need me to help?
<wgrant> bac: They're all approved and all but one is tested, but PQM rejected three of them due to testfix.
<bac> wgrant: i'll be glad to help but i don't want to duplicate effort.  why don't you send me an email with the URLs of the MPs and what you need me to do for each.  please copy noodles775 and mwhudson.
<bac> i'll then land them when we're out of testfix
<jml> ... which should be any moment now-
<noodles775> bac, wgrant : I've sent off ~wgrant/launchpad/bug-549907-stop-using-slave-build-id a few hours ago too.
<noodles775> (still playing)
 * bac wishes he could send a branch off for testing and landing from the MP page
<wgrant> PQM is alive?
<wgrant> If it is... can someone please land https://code.edge.launchpad.net/~wgrant/launchpad/bug-538844-master-side/+merge/22681, https://code.edge.launchpad.net/~wgrant/launchpad/more-buildmaster-cleanup/+merge/22735 and https://code.edge.launchpad.net/~wgrant/launchpad/remove-dbnote/+merge/22736? They all passed ec2test a few hours ago (with the current devel rev), but were rejected by PQM.
<noodles775> Sure. Hrm, how do I land something *not* via ec2, without branching?
 * noodles775 looks at ec2 land code.
<wgrant> noodles775: bzr lp-land, maybe?
<noodles775> wgrant: yep, was just reading bzr help lp-land, thanks!
<wgrant> Not sure if that handles generating the [r=foo] stuff, though.
<noodles775> ugh, bzr: ERROR: exceptions.ValueError: Cannot determine Bazaar host. "https://api.edge.launchpad.net/beta/" not a recognized Launchpad API root.
<wgrant> Yay, version-naÃ¯ve code.
 * noodles775 updates pqm plugin
<noodles775> OK, that helped, but now it complains about lacking revisions... I'm guessing lp-land still requires a local branch?
<noodles775> bzr: ERROR: Public branch "bzr+ssh://bazaar.launchpad.net/~wgrant/launchpad/bug-538844-master-side" lacks revision "launchpad@pqm.canonical.com-20100407001252-2bvaz1hi60knsrnp".
 * noodles775 branches and tries.
* noodles775 changed the topic of #launchpad-reviews to: On call: - || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* rockstar changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<rockstar> abentley, can you do a review (maybe two) for me?
<abentley> rockstar, sure.  Working up the description for my merge request right now.
<rockstar> abentley, great.
<rockstar> abentley, here's the easy one: https://code.edge.launchpad.net/~rockstar/launchpad/ihasrecipes-person/+merge/23045
<rockstar> abentley, the next one still needs changes since I'm not making DistroSeries implement IHasRecipes
<abentley> rockstar, I recommend using lp-propose (nÃ©e lp-submit) and the lp_review_body plugin.
<rockstar> abentley, yeah, I've been meaning to set that up.
<abentley> rockstar, maybe I'll walk you through that on our next call.
<abentley> rockstar, test_person_implements_hasbranches looks misnamed.
<abentley> rockstar, similarly test_product_implements_hasbranches
<abentley> rockstar, test_product_getRecipes comment looks wrong.
<abentley> rockstar, docstring in lib/lp/registry/model/product.py is wrong.
<rockstar> abentley, argh. c-n-p errors...
<abentley> rockstar, originally you were going to try adaptation, right?  What problems did you run into with that?
<rockstar> abentley, the fact that I wanted to get something done and not fart around with zope oddities.
<rockstar> The docstring was a thinko, not a c-n-p error.  *facepalm*
<abentley> rockstar, adding coupling between Person and Recipe seems like tech debt.  could you file a bug to reexamine it?
<rockstar> abentley, what do you mean "adding coupling" ?
<abentley> rockstar, Person (and Product, I guess) now have to know what a Recipe is.
<abentley> There is a dependency now that wasn't there before.
<rockstar> abentley, yeah, I guess you're right.  I'll file a bug and say "let's think about adapters again"
<abentley> rockstar, cool.
<rockstar> abentley, by that reasoning though, IHasBranches should probably also be an adapter.
<abentley> rockstar, yes, there should be an adapter for Person that provides IHasBranches.
<rockstar> abentley, yeah, I bet that would happen more often if zope wasn't stingy.  :)
<abentley> rockstar, at least in the ideal, registry doesn't depend on us and we don't depend on other apps like Soyuz.
 * rockstar nods
<sinzui> abentley, yes, and we must do better at decoupling the features
<rockstar> sinzui, we aren't in the habit of it.  I think that's what the problem really is.
<abentley> rockstar, could you ping me when you've updated those doc errors?
<rockstar> We haven't really focused on not coupling features.
<sinzui> rockstar, indeed. It is easier to see the problem now that we have separate trees
<rockstar> abentley, pushed, waiting for scanner...
<rockstar> sinzui, yeah, that is helpful.
<abentley> rockstar, cool.  I'll watch it then.
<rockstar> It would probably reduce the risk of circular imports by an order of magnitude also.
<rockstar> abentley, just noticed one of the comments is still wrong...  :/
 * rockstar waits for scanner again.
<rockstar> abentley, alright, everything should be kosher now.
<abentley> rockstar, looking...
<abentley> rockstar, products can have sourcepackage branches, but your product.getRecipes() doesn't include them, right?
<rockstar> abentley, good question.  I have no idea.
<rockstar> abentley, I was under the assumption that querying the Branch table would bring them all in.
<rockstar> If that's not the case, "eep."
<abentley> I don't think it would do that.
<abentley> I thought you were deliberately excluding them.
<rockstar> abentley, I don't know why I would deliberately exclude them.
<abentley> AIUI, sourcepackagebranches do not necessarily have a product.
<rockstar> abentley, ah, well, in that case, they wouldn't come up associated with the product then.
<abentley> You need to have a link between the sourcepackage and the product.
<abentley> And then to find sourcepackagebranches, you'd need to find the branches whose sourcepackage was linked to that product.
<rockstar> abentley, hm, I question whether or not it matters then.  If the source package doesn't have a product, than it shouldn't show up by product anyway.
<rockstar> I suspect we'll probably want related recipes for a source package, but I wasn't going to implement that until someone wanted it.
<abentley> rockstar, if you want to exclude sourcepackagebranches for now, you need to tweak your comment slightly.
<abentley> "IProduct.recipes should provide all the SourcePackageRecipes attached to that product." => "IProduct.recipes should provide all the SourcePackageRecipes attached to that product's branches"
<rockstar> abentley, Sure.
<rockstar> abentley, done.
<abentley> rockstar, r=me.
<rockstar> abentley, cheers.
<abentley> rockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/answer-ordering/+merge/23050?
<abentley> rockstar, Also, request-build is ready for review: https://code.edge.launchpad.net/~abentley/launchpad/request-build/+merge/22570
<rockstar> abentley, gotcha.  I'm on it.
<rockstar> abentley, r=me on the ordering one, looking at the other one now.
<rockstar> abentley, what's up with pylint now?
<abentley> rockstar, I don't know.
<rockstar> abentley, I wonder why we still have pylint when it never seems to do anything but give us false positives.
#launchpad-reviews 2010-04-09
<jelmer> rockstar: Hi! If you're still reviewing, any chance you can have a look at this mp: https://code.launchpad.net/~jelmer/launchpad/upgrade-sourcecode/+merge/23034
* sinzui changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<sinzui> rockstar, thumper: can either of you review https://code.launchpad.net/~sinzui/launchpad/series-branch-target/+merge/23056
<wgrant> Can someone please land https://code.edge.launchpad.net/~wgrant/launchpad/move-rescueiflost-tests/+merge/22737?
<mwhudson> man it's all ask ask ask in here today
<mwhudson> jelmer: did you consider just deleting and re-pulling in the cross-format case?
<mwhudson> it's simpler and probably quicker
<mwhudson> wgrant: is this one of the ones that passed ec2 yesterday but got foiled by testfix?
<wgrant> mwhudson: No, noodles landed all of them overnight.
<wgrant> This one needs EC2ing too.
<mwhudson> wgrant: OK
<jelmer> mwhudson: that's a good point; do you reckon that's a better idea than upgrading? I guess it's a bandwidth vs cpu tradeoff
<mwhudson> jelmer: my thinking is that they're all small branches so that simplicity is better
<jelmer> mwhudson: I wonder if removing the branch is simpler in this case though; it'll involve checking that the tree is unchanged, etc
<mwhudson> jelmer: hm, fair point
<mwhudson> jelmer: you seem to have lost the 'don't pull if the revision id is the same' thing
<mwhudson> but i guess that's not much of an optimization really
<jelmer> mwhudson: Bazaar really should be taking care of that case itself
<jelmer> at least imnsho :-)
<mwhudson> quite
<mwhudson> jelmer: approved
<jelmer> mwhudson: thanks!
<mwhudson> wgrant: your branch has conflicts with devel it seems
<wgrant> mwhudson: Bah.
 * wgrant merges.
 * jpds has https://code.launchpad.net/~jpds/launchpad/stormify-distribution-bits/+merge/23060 up for grabs.
* jpds changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [sinzui,jpds] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<mwhudson> wgrant: pushed yet?  should i run ec2 land again?
<wgrant> mwhudson: Just fighting buildout so I can rerun the involved tests.
<mwhudson> heh heh
 * wgrant stabs $SOMETHING in the face.
<wgrant> lazr.config.interfaces.NoConfigError: No config with name: initZopeless config overlay.
<wgrant> YES BUT WHY?
<jpds>  /11
<wgrant> So it turns out that the error actually meant that I needed to fix permissions on the account table.
<wgrant> That makes sense.
<wgrant> mwhudson: Pushed.
<wgrant> mwhudson: Did you kick that ec2 off yet?
<mwhudson> wgrant: yeah, sorry
<mwhudson> wgrant: it's headless now
<wgrant> Thanks.
<wgrant> Great.
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: jpds || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<adeuring> On call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<mwhudson> adeuring: you missed a /topic there
<adeuring> mwhudson: arghh... thanks!
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<gmb> adeuring, Morning. Can you give me a code review on https://code.edge.launchpad.net/~gmb/launchpad/add-next_check-to-bugwatch-pages-bug-558410/+merge/23080?
<adeuring> gmb: sure
<gmb> noodles775, Any chance of a quick UI review of https://code.edge.launchpad.net/~gmb/launchpad/add-next_check-to-bugwatch-pages-bug-558410/+merge/23080? I can provide screenshots if needed; it's a trivial change.
<noodles775> gmb: I can take a look, but if it's non-urgent, it'd be great to get one of the others (rockstar, for eg), to do an initial review and I'll do the second one?
<noodles775> Although, if it's that trivial, might not be worth it.
<gmb> noodles775, Well, it's just a case of adding data to the BugWatch +edit and BugTracker overview pages (lastchecked and next_check dates for bug watches, basically)
<gmb> So, your call.
<noodles775> OK, I'll just do it then.
<gmb> Thanks.
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: gmb || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<adeuring> gmb: r=me
<gmb> noodles775, FTR, I have no idea why I've called next_check "Scheduled" on BugWatch+edit but called it "Next check" on the BugTracker overview. I think we should probably stick to one label, agreed?
<gmb> adeuring, Thanks
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: gmb || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<deryck> Hi adeuring.  I have one for the queue, when you can look at it.  https://code.edge.launchpad.net/~deryck/launchpad/not-notified-someone-else-subscribed-494257/+merge/23044
* deryck changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [sinzui, deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<adeuring> deryck: surw, I'll look
<deryck> adeuring, thanks!
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: deryck || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<adeuring> deryck: overall, you branch looks good. I just wondered if it makes sense to explicitly test that bug.subscribe(..., send_notifications=False) does indeed not send notifications.
<deryck> adeuring, ah, good point.
<adeuring> deryck: OK, so r=me
<deryck> adeuring, thanks!  I'll add the test here in a moment.
<gmb> noodles775, Thanks for the review.
<noodles775> np.
<maxb> Hello OCR, I'd like to enqueue https://code.edge.launchpad.net/~maxb/launchpad/ignored-asserts/+merge/18889 please, which is up for re-review after a tweak to the tests.
* maxb changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: deryck || queue: [sinzui,maxb(ignored-asserts)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<adeuring> maxb: sure, I'll look.
<adeuring> maxb: Or do you want gmb to ciÃ³ntinue the review=
<adeuring> s/=/?/
<maxb> In this case, I don't think there's enough context for it to be worth me seeking out gmb - I'd rather get it reviewed today :-)
<adeuring> fair eonough
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: maxb(ignored-asserts) || queue: [sinzui,] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<maxb> Ah, I see gmb was around earlier - I only looked at the fact that he is currently marked away. Never mind, there really isn't much prior context to this branch.
<adeuring> maxb: r=me. As Graham said, good catch!
<maxb> adeuring: Thanks, could you land it for me?
<adeuring> maxb: sure
<maxb> excellent. one more baby step towards python 2.6
<maxb> oh, I'm supposed to set a commit message in the MP, aren't I
<maxb> done
* sinzui changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: maxb(ignored-asserts) || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<gary_poster> adeuring: could you look at https://code.edge.launchpad.net/~gary/launchpad/small/+merge/23052 ?  This was a quick fix for a problem abentley found on karmic
<adeuring> gary_poster: sure
<gary_poster> thank you
<abentley> gary_poster, adeuring: fix verified on my system.
<gary_poster> thanks abentley
<adeuring> gary_poster: r=me
<gary_poster> thank you adeuring!
<jpds> adeuring: Hi, did you ec2 land my branch?
<adeuring> jpds: sorry, forgot it... Will do that now.
<jml> adeuring: could you please review https://code.edge.launchpad.net/~jml/launchpad/more-services/+merge/23107 â it's a branch with a few simple cleanups?
<adeuring> jml: sure
<jml> thanks.
<jpds> adeuring: No problem, thanks!
<allenap> adeuring: Could you rubber-stamp a proposal for me please? It's a cherry-pick of an already-reviewed change. https://code.edge.launchpad.net/~allenap/launchpad/dynamic-batch-size-bug-546085-devel/+merge/23105
<adeuring> allenap: done
<allenap> adeuring: Thanks :)
<allenap> adeuring: Could you try changing the review type to "code", or add an approve for "code" (or nothing)? bzr lp-land complains at the moment. (And if that doesn't work I'll land it by hand.)
<adeuring> allenap: done. (sorry for the delay -- was on skype)
<allenap> adeuring: Thanks :)
<adeuring> jml: r=me
<jml> adeuring, thanks.
<allenap> adeuring: Do you have time for another? Should be straightforward I hope. https://code.edge.launchpad.net/~allenap/launchpad/dont-update-dupe-bugs-bug-511276/+merge/23113
<adeuring> allenap: yes
<adeuring> allenap: r=me
<allenap> adeuring: Thanks!
<allenap> adeuring: I've got a silly one for you now :) https://code.edge.launchpad.net/~allenap/launchpad/dynamic-batch-size-bug-546085-db-devel-preempt-conflict/+merge/23114
<adeuring> allenap: rs=me
<allenap> adeuring: Thanks :)
* deryck changed the topic of #launchpad-reviews to: On call: || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
<deryck> abel is gone.  Is anyone else reviewing this afternoon?
<deryck> rockstar or abentley -- could I ask a review from one of you fine gentleman?  It's to do with linking branches on a bug page.
<abentley> deryck, sure.
<deryck> abentley, thanks!  https://code.edge.launchpad.net/~deryck/launchpad/no-linky-abandoned-branches-559325/+merge/23135
<abentley> deryck, was there a preimplementation call?
<deryck> abentley, no.  I just did it on a whim at lunch.
<maxb> deryck: I question the appropriateness of this change because it hides potential partial work done on a bug that someone else could rescue and complete
<abentley> deryck, I'm not sure this is the right solution.
<deryck> abentley, ah, ok.  fair enough.  I did discuss the proposed solution with jml in the bug.  And he agreed with the approach.
<abentley> deryck, yes, I saw that.  Still, he's not on the code team anymore.
<abentley> deryck, my reasons are much like maxb's.
<deryck> abentley, I didn't see maxb's comment?  Where is that?
<abentley> deryck, and I'm not sure that abandoning a branch is a clear indication that you want to never get any questions about it.
<abentley> deryck, in channel, 4 minutes ago.
<deryck> ah, sorry. thought that was you :-)
<deryck> the red bled together in xchat :-)
<abentley> perhaps appropriate in this case.
<deryck> abentley, so your argument would be if you don't want the connection then unlink the branch?
<abentley> deryck, that seems suboptimal too.
<abentley> deryck, is it true that you never want to receive any questions about the branch, or do you just not want to be asked whether you're working on it?
<maxb> Would it not be sufficient to display the branch's "Abandoned" status alongside its name in the bug "Related branches" list?
<deryck> abentley, I would guess more the latter.  But I think it's hard to presume what anyone means by "abandoned" really.
<deryck> maxb, it's already listed.
<deryck> maxb, sorry, I'm wrong
<deryck> was thinking of rejected.
<maxb> MP status is shown, branch status is nort
<maxb> *not
<deryck> right
<deryck> yeah, maybe that's the right fix.
<abentley> deryck, so I think branch status should be shown, and we should maybe reduce the visibility of the merge proposal.
<abentley> deryck, but I'd be interested to hear what rockstar thinks.
<maxb> I like the idea of reducing the visibility of the MP
<deryck> abentley, I can go along with your suggestion.  but would be interested in rockstar's opinion, too.  I can go either way.  It makes sense to me to drop it completely, too.
<deryck> but I'm thinking as a bugs guy obviously :-)
<maxb> Drop what? the MP or the entire branch?
<deryck> entire branch per my fix now
<maxb> Just because someone has abandoned a branch does not mean that it does not contain useful work
<abentley> deryck, if I'm not mistaken, the reason the branch status isn't shown is because the MP *is* shown.
<deryck> maxb, I don't disagree with you.  I'm saying I can see both sides.  Abandoned branches could also be useless, as well as they could be useful.
<maxb> Better to err on the side of not losing visibility on potential useful code, IMO
<deryck> abentley, ok.  I don't think branch status is really useful unless it's abandoned.  So if we go this route, maybe we should treated abandoned branches differently, i.e. show branch status and a link to MP but no info on the MP?
<deryck> maxb, true.  but my assumption is someone marked it "abandoned" precisely because it wasn't useful.
<abentley> deryck, we use the "merged" status very heavily.  Others not so much.
<maxb> "I have stopped work on this branch" doesn't mean the same as "There's no useful work in this branch"
<abentley> deryck, that's not what I would assume.  I would assume they didn't have time for it, or maybe the reviewer was being a jerk and obstructing it.
<deryck> maxb, but does ABANDONED always equal "I have stopped work on this branch?"  If so, how do you know that?  :-)  That's all I mean.  I'm not intending to argue with you, really. :-)
<abentley> deryck, yes, that's what it means.
<maxb> Well, that's what the English word means to me :-)
<deryck> fair enough.  I take your points.  But I don't have a desire to pursue it beyond this.  it was a lunch time fix for me.  I like removing some clutter from the bug page, but if we don't agree is right, I won't pursue it further myself.
<deryck> Thanks anyway, abentley, for the review.
<abentley> deryck, very well.  Sorry it didn't work out.
<deryck> no worries
 * rockstar sees that he was apparently pinged in this channel while at lunch
<abentley> rockstar, don't worry about it.
<rockstar> abentley, yeah, it looks like there was consensus.
<abentley> rockstar, well, more like resolution.
<rockstar> abentley, I'd be all for hiding abandoned branches on a bug that have a branch that actually fixed the bug (and was landed)
<rockstar> In that case, I'd only show the one that fixed the issue.
<abentley> rockstar, chat?
<rockstar> abentley, oh, it's getting to be that time huh?
<rockstar> abentley, can you give me 5 minutes to get my tests passing?
<abentley> rockstar, sure.
