User1: :-)
User1: al-maisan_: Will there be BuildQueue records that reference a Job that does not have a corresponding BuildPackageJob record?
User1: It is probably better to link BuildQueue to BuildPackageJob then rather than directly to Job. That will enforce that better.
User1: Ok. So in the future a BuildQueue record may be linked to a Job without a BuildPackageJob.
User1: RosettaJob sounds like a dodgy table name, but we can worry about that when it is suggested. Patch looks good.
User2: gmb or bac, i have a trivial branch for you: https://code.edge.launchpad.net/~leonardr/lazr.restful/474522-unmarshall-uri/+merge/14691
User3: User2 i can take it
* bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: -,leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com
User3: User2 done.  thanks.
* bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: -,- || queue [] || This channel is logged: http://irclogs.ubuntu.com
User4: User5 Fancy a review? :) https://code.edge.launchpad.net/~allenap/launchpad/ec2-parry/+merge/14693
User4: User5 There are 4 diffs in that review. If you could do one of them I'd be mucho grateful.
User5: User4 Sure, I'll take one.
User5: User4 Wouldn't it be easier to use bzr pipeline to split it up into different branches, or is the change pretty much atomic?
User5: Anyway, I'll take the first diff to start with.
* gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: allenap-1, - || queue [allenap-2, allenap-3, allenap-4] || This channel is logged: http://irclogs.ubuntu.com
User4: User5 I probably should. I'm happy to do it if you want?
User5: User4 Might be worth giving it a shot, otherwise the m-p is going to get confusing.
User4: User5 The first diff is the biggest, the most complex, but probably the most interesting.
User5: I'll keep looking over it, then :)
User4: User5 I'll split it up too. Worth learning if nothing else (I used the predecessor to pipeline once and got very very confused).
User5: Didn't we all?
User5: User4 Your XXX about permissiveness should have a bug filed for it.
User4: User5 Ah yes... there are a lot of XXXs in there. I shall file bugs for all of them.
User5: Cool.
* sinzui changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: allenap-1, - || queue [allenap-2, allenap-3, allenap-4, sinzui] || This channel is logged: http://irclogs.ubuntu.com
User5: User4 Your first diff looks good. Once you've split it up, request a review from me and I'll approve it.
User4: User5 Cool, thanks :)
User5: User3 I've got to go afk for a while to pick my missus up from work. I'll come back online later and finish my review shift then, if you haven't powered through them all ;)
* gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: -, - || queue [allenap-2, allenap-3, allenap-4, sinzui] || This channel is logged: http://irclogs.ubuntu.com
 * gmb -> exeunt
User3: User5  great
User2: bac,can you take a look at this trivial backport of my branch which you reviewed earlier?
User2: https://code.edge.launchpad.net/~leonardr/lazr.restful/474522-oops-backport/+merge/14696
User3: User6 when you get a chance: https://code.edge.launchpad.net/~bac/launchpad/bug-476833/+merge/14697
User3: User2 sure
User6: User3 I start it in a few minutes
User3: User6 no rush.  i'm going to lunch soon
User3: User2 done
User2: great
* bac changed the topic of #launchpad-reviews to: on call: bac-lunch || reviewing: -, - || queue [allenap-2, allenap-3, allenap-4, sinzui] || This channel is logged: http://irclogs.ubuntu.com
* bigjools changed the topic of #launchpad-reviews to: on call: bac-lunch || reviewing: -, - || queue [allenap-2, allenap-3, allenap-4, sinzui, bigjools (CP candidate)] || This channel is logged: http://irclogs.ubuntu.com
* bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: -, - || queue [allenap-2, allenap-3, allenap-4, sinzui, bigjools (CP candidate)] || This channel is logged: http://irclogs.ubuntu.com
* bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue [allenap-2, allenap-3, allenap-4, sinzui, bigjools (CP candidate)] || This channel is logged: http://irclogs.ubuntu.com
* bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: bigjools || queue [allenap-2, allenap-3, allenap-4, sinzui] || This channel is logged: http://irclogs.ubuntu.com
User3: hi bigjools
User7: hi bac
User3: User7 just looking at your branch
User7: great
User3: User7 if you change line 85 to be
User3: for build in builds_list{:num_builds]
User3: you can get rid of the break logic
User7: true
User3: also, what about exporting the magic number of 80 to a symbol, since you say it will likely change in the future
User7: User3 it will almost certainly change - to a completely different algorithm
User3: other than those small suggestions it looks good
User7: this is a stopgap measure
User7: how'd ya like the monster query? :)
User3: User7 ah, ok.  i just find those hard-code values to be cringey
User3: eek
User7: yeah I know, I didn't really like it either, but I figured since it was going to get removed ...
User7: but I don't mind changing it if you prefer
User3: User7 just do it if you have to touch this again.  then you'll appreciate it.
User7: okay
User7: not a problem
User3: looks good.  let me do it up official
User7: ok, thanks bac, I'll land it later tonight my time, I gotta go now
User3: ok
* bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: sinzui || queue [allenap-2, allenap-3, allenap-4] || This channel is logged: http://irclogs.ubuntu.com
User3: User6 this is the branch you queued up:  https://code.edge.launchpad.net/~sinzui/launchpad/distroseries-breadcrumb-bug-400643/+merge/14657  ?
User6: User3 yes. Sorry for 500 lines of mechanical changes
User3: User6 np
User6: We have got to get rid of those unused and untested application search options
 * sinzui considers the Christmas holidays to remove the badness
User6: User3 r=me as it is. I have no remarks other than thanks for the comprehensive tests
User3: User6 cool, thanks
User6: User8 I have some questions for you at https://code.edge.launchpad.net/~rockstar/launchpad/inline-mp-status/+merge/14700? I think there are some improvements you can make to the code and a have a few questions. I think you can answers my questions quickly
 * rockstar looks
User8: sinzui, okay, so there was some lint and that commented out error handler that were removed after you saw the patch.
User8: The patch should be updated to show that.
User6: fab
User8: sinzui, also, the lazr-js widgets make baby jesus cry because they use ids.  The branch index page currently has this bug, and we'll fix that this week.
 * sinzui nods
User8: sinzui, technically, that "handcrafted" link is tested, since it adds a class that the widget needs to be able to find.  The windmill test then tests it.
User6: So we are certain that enabled works under all conditions and that is the correct text?
User8: sinzui, and I see no reason why this javascript shouldn't be compressed.  It's just that I didn't know about that base-layout-macros.
User8: sinzui, oh, I see what you're saying.  That is tested well in pagetests.
User8: The only thing I changed was adding a class.
User6: User8 The list of scripts used to be in main-template. The moved to macros last June
User8: sinzui, okay, so where is base-layout-macros?
User6: Why are we testing something defined in a view in a page test? Are you testing that the link is there and then not there for all scales of permission? Are we uncertain the @require('launchpad.Edit') works?
User6: lib/lp/app/templates/base-layout-macros.pt
User8: sinzui, I'm pretty sure that link is tested properly.  If it's not, it's still out of the scope of this branch.
User6: User8 we have a formatter so that we never have to ask these questions.
User6: if the formatter does not work, we should fix it
User8: sinzui, yes, but the formatter does not provide for being able to set ids of the links.  This is a known problem.
User6: for example, what if we change the icon of the link, so we want to change every template that duplicated the instruction
User6: User8 that link does not have an id. I am confused
User8: I am confused too.  WTF...
User6: User8 the difference between that template and the macro is that macro also guarantees not extra whitespace is generated
User8: sinzui, is that really a problem?
User6: We had s lot of mismatched icons during the 3.0 conversion because they were hard coded.
User6: User8 please update this link to the standard. when you use the formatter, you know you do not need to test the icon, the text, or if it is enabled.
User8: sinzui, the formatter doesn't let me set the class of the edit icon, does it?
 * rockstar doesn't see.
User6: User8 it sets the <a> link class, then created a <span class="invisible-link">link text</span>
User6: There is no icon, we use sprites
User8: sinzui, yes, but the class on the icon is important to the widget.
User6: okay now we are making progress
User8: So we didn't need the handcrafted link before, but we do now.
User6: Does beuno know that his maoist death march to sprites is being undermined by the Chang Kai Shecks' nationalist lazr.js force
User6: User8 I leave the link as it is since as you point out, it is required.
User6: User8 I think we are done
User8: sinzui, thanks.
User6: User8 r=me
User8: sinzui, I do think we need to re-consider that kind of markup that lazr-js widgets need.
User6: User8 I have a branch that fixed some UI issues in blueprints. can you take a look at the QA section in the proposal and the images I added in a comment: https://code.edge.launchpad.net/~sinzui/launchpad/blueprints-ui/+merge/14639
User2: User6 can i jump the allenap-ful queue with a tiny branch that just upgrades some launchpad packages?
User2: https://code.edge.launchpad.net/~leonardr/launchpad/launchpadlib-devel/+merge/14705
User6: User2 certainly with me. I am not an OCR
 * sinzui looks
User2: argh
User2: the topic scrolled off my screen and so "sinzui" was the first name i saw
User6: np
User2: but you might want to review that anyway since it fixes the problem you were complaining about
User6: indeed I know the issue
User6: I am just waiting for the diff to appear
* bac changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [allenap-2, allenap-3, allenap-4] || This channel is logged: http://irclogs.ubuntu.com
User3: User6 are you reviewing leonardr or do you want me to
User6: I will review it
User3: User6 we've spent more time discussing who gets to review it than it'll take.  i'm glad to see it land!
User6: indeed!
User6: User2 land your branch
User2: ok
User6: User3 are you still about for a review
User3: User6 we're on our way out the door.
User6: okay
User3: is it short?
User6: The fix is, I got consumed in by sorting the imports https://code.edge.launchpad.net/~sinzui/launchpad/active-series-sort-bug-464014/+merge/14720
User6: bac I created series like lucid 10.04 and verify we sort it on the DSP page
User3: User6 i'll have a look now
User9: User6 up to relook at the project page change?
User9: User6 it has a test now :)
User6: sure
User3: User6 done.  nice.
 * bac EOD & off to dinner
User6: fab: I can make some people happy
User6: User9 lines 87-91 are like many tests that failed because there is no confirmation or the confirmation is delayed until it is not clear what went wrong. You can fix this and justify the use of a story by completing the story, The owner should not be opening his browser on line 113, if he is not already on the correct page, lines 87-91 failed.
User6: I think you can fix this my switching the order of 102 and 113 and removing the call for owner_browser.open('http://launchpad.dev/fooix')
User9: User6 I think the reason the browser is opened, is that once the branch is linked, it ends up on the product series page, not the project page
User9: User6 we could add yet another view that knows to go back to the main page
User6: User9 not think, we must know. That view should return the user, that is part of the story
User9: User6 I'm open to suggestions
User6: if you want to test state write a doctest or unittest
User9: User6 so, what do you suggest for the story?
User9: User6 write it as it happens
User9: User6 or change what happens
User6: changing the order will make a simple narrative instead of something from a David Lynch movie
User6: owner seeing nothing, he uses the edit link, he submits the form, he sees it worked
User6: anon visits and sees the information too
User6: User9 my only remark about the template change is line 149. I expected to see a "sprite" with 'info', then I asked why there isn't a menu link for this? Is there a menu link to browse the source code?
User9: probably not
User9: there isn't always a link
User6: that is disappointing
User9: how do we handle sometimes menu items?
User9: are you meaning on the menu for the branch?
User6: thumper the only uses of class="info" are on lists, there are only three of them, and I believe I have a branch that removed two of them
User9: hmm
User6: User9 I was hoping for a menu item so that you did not have to make the sprite
User6: User9 I see lots of class="sprite info"
User9: I can change it
User6: User9 I think we are done here. fix the link and reorder the test.
User9: User6 ok
launchpad-reviews
Ubuntu IRC