| === Ursinha is now known as Ursinha-afk | ||
| === flacoste_lunch is now known as flacoste | ||
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: sinzui || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com | ||
| === bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: sinzui || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com | ||
| === Ursinha-afk is now known as Ursinha | ||
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: lunch || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: lunch || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| al-maisan_ | stub: I'd appreciate it if you could look at https://code.edge.launchpad.net/~al-maisan/launchpad/builddsc-478919/+merge/14689 at your earliest convenience. | 12:03 |
|---|---|---|
| al-maisan_ | stub: this patch should be much more readable as I promised the last time ;) | 12:04 |
| stub | :-) | 12:04 |
| stub | al-maisan_: Will there be BuildQueue records that reference a Job that does not have a corresponding BuildPackageJob record? | 12:11 |
| al-maisan_ | stub: no, they always come in pairs | 12:12 |
| al-maisan_ | Job and BuildPackageJob always come in pairs that is | 12:12 |
| stub | It is probably better to link BuildQueue to BuildPackageJob then rather than directly to Job. That will enforce that better. | 12:12 |
| al-maisan_ | stub: the issue here is that there will be other types of jobs (in addition to BuildPackageJob) later | 12:13 |
| stub | Ok. So in the future a BuildQueue record may be linked to a Job without a BuildPackageJob. | 12:14 |
| al-maisan_ | that's why BuildQueue refers to Job as opposed to some of these specific rows | 12:14 |
| al-maisan_ | stub: there will be e.g. a combination of Job and BuildFromBranchJob | 12:15 |
| al-maisan_ | .. or of Job and RosettaJob | 12:15 |
| al-maisan_ | hance BuildQueue refers to Job and not to any of: BuildPackageJob, BuildFromBranchJob or RosettaJob | 12:16 |
| al-maisan_ | directly | 12:16 |
| stub | RosettaJob sounds like a dodgy table name, but we can worry about that when it is suggested. Patch looks good. | 12:26 |
| al-maisan_ | stub: 'RosettaJob' was just an example. Thank you for reviewing it! | 12:34 |
| === matsubara_ is now known as matsubara | ||
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| leonardr | gmb or bac, i have a trivial branch for you: https://code.edge.launchpad.net/~leonardr/lazr.restful/474522-unmarshall-uri/+merge/14691 | 14:01 |
| bac | leonardr: i can take it | 14:01 |
| === bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: -,leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| bac | leonardr: done. thanks. | 14:22 |
| === bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: -,- || queue [] || This channel is logged: http://irclogs.ubuntu.com | ||
| allenap | gmb: Fancy a review? :) https://code.edge.launchpad.net/~allenap/launchpad/ec2-parry/+merge/14693 | 15:03 |
| allenap | gmb: There are 4 diffs in that review. If you could do one of them I'd be mucho grateful. | 15:04 |
| gmb | allenap: Sure, I'll take one. | 15:07 |
| gmb | allenap: Wouldn't it be easier to use bzr pipeline to split it up into different branches, or is the change pretty much atomic? | 15:07 |
| gmb | Anyway, I'll take the first diff to start with. | 15:08 |
| === 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 | ||
| allenap | gmb: I probably should. I'm happy to do it if you want? | 15:14 |
| gmb | allenap: Might be worth giving it a shot, otherwise the m-p is going to get confusing. | 15:14 |
| allenap | gmb: The first diff is the biggest, the most complex, but probably the most interesting. | 15:14 |
| gmb | I'll keep looking over it, then :) | 15:14 |
| allenap | gmb: I'll split it up too. Worth learning if nothing else (I used the predecessor to pipeline once and got very very confused). | 15:15 |
| gmb | Didn't we all? | 15:15 |
| gmb | allenap: Your XXX about permissiveness should have a bug filed for it. | 15:32 |
| allenap | gmb: Ah yes... there are a lot of XXXs in there. I shall file bugs for all of them. | 15:33 |
| gmb | Cool. | 15:33 |
| === 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 | ||
| gmb | allenap: Your first diff looks good. Once you've split it up, request a review from me and I'll approve it. | 15:44 |
| allenap | gmb: Cool, thanks :) | 15:44 |
| gmb | bac: 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 ;) | 15:44 |
| === 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 | 15:45 | |
| bac | gmb: great | 15:45 |
| leonardr | bac,can you take a look at this trivial backport of my branch which you reviewed earlier? | 15:56 |
| leonardr | https://code.edge.launchpad.net/~leonardr/lazr.restful/474522-oops-backport/+merge/14696 | 15:56 |
| bac | sinzui: when you get a chance: https://code.edge.launchpad.net/~bac/launchpad/bug-476833/+merge/14697 | 16:08 |
| bac | leonardr: sure | 16:08 |
| sinzui | bac: I start it in a few minutes | 16:09 |
| bac | sinzui: no rush. i'm going to lunch soon | 16:09 |
| bac | leonardr: done | 16:10 |
| leonardr | great | 16:10 |
| === 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 | ||
| bac | hi bigjools | 17:44 |
| bigjools | hi bac | 17:45 |
| bac | bigjools: just looking at your branch | 17:45 |
| bigjools | great | 17:45 |
| bac | bigjools: if you change line 85 to be | 17:46 |
| bac | for build in builds_list{:num_builds] | 17:46 |
| bac | you can get rid of the break logic | 17:46 |
| bigjools | true | 17:46 |
| bac | also, what about exporting the magic number of 80 to a symbol, since you say it will likely change in the future | 17:47 |
| bigjools | bac: it will almost certainly change - to a completely different algorithm | 17:47 |
| bac | other than those small suggestions it looks good | 17:47 |
| bigjools | this is a stopgap measure | 17:47 |
| bigjools | how'd ya like the monster query? :) | 17:47 |
| bac | bigjools: ah, ok. i just find those hard-code values to be cringey | 17:47 |
| bac | eek | 17:48 |
| bigjools | yeah I know, I didn't really like it either, but I figured since it was going to get removed ... | 17:48 |
| bigjools | but I don't mind changing it if you prefer | 17:48 |
| bac | bigjools: just do it if you have to touch this again. then you'll appreciate it. | 17:49 |
| bigjools | okay | 17:49 |
| bigjools | not a problem | 17:49 |
| bac | looks good. let me do it up official | 17:49 |
| bigjools | ok, thanks bac, I'll land it later tonight my time, I gotta go now | 17:49 |
| bac | ok | 17:50 |
| === 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 | ||
| bac | sinzui: this is the branch you queued up: https://code.edge.launchpad.net/~sinzui/launchpad/distroseries-breadcrumb-bug-400643/+merge/14657 ? | 17:56 |
| sinzui | bac: yes. Sorry for 500 lines of mechanical changes | 17:57 |
| bac | sinzui: np | 17:57 |
| sinzui | We have got to get rid of those unused and untested application search options | 18:00 |
| * sinzui considers the Christmas holidays to remove the badness | 18:00 | |
| sinzui | bac: r=me as it is. I have no remarks other than thanks for the comprehensive tests | 18:01 |
| bac | sinzui: cool, thanks | 18:01 |
| === henninge_ is now known as henninge | ||
| sinzui | rockstar: 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 | 18:33 |
| * rockstar looks | 18:33 | |
| rockstar | sinzui, okay, so there was some lint and that commented out error handler that were removed after you saw the patch. | 18:36 |
| rockstar | The patch should be updated to show that. | 18:36 |
| sinzui | fab | 18:37 |
| rockstar | 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. | 18:38 |
| * sinzui nods | 18:39 | |
| rockstar | 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. | 18:42 |
| sinzui | So we are certain that enabled works under all conditions and that is the correct text? | 18:43 |
| rockstar | 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. | 18:43 |
| rockstar | sinzui, oh, I see what you're saying. That is tested well in pagetests. | 18:44 |
| rockstar | The only thing I changed was adding a class. | 18:44 |
| sinzui | rockstar: The list of scripts used to be in main-template. The moved to macros last June | 18:44 |
| rockstar | sinzui, okay, so where is base-layout-macros? | 18:45 |
| sinzui | 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? | 18:46 |
| sinzui | lib/lp/app/templates/base-layout-macros.pt | 18:46 |
| rockstar | sinzui, I'm pretty sure that link is tested properly. If it's not, it's still out of the scope of this branch. | 18:47 |
| sinzui | rockstar: we have a formatter so that we never have to ask these questions. | 18:48 |
| sinzui | if the formatter does not work, we should fix it | 18:48 |
| rockstar | sinzui, yes, but the formatter does not provide for being able to set ids of the links. This is a known problem. | 18:49 |
| sinzui | for example, what if we change the icon of the link, so we want to change every template that duplicated the instruction | 18:49 |
| sinzui | rockstar: that link does not have an id. I am confused | 18:50 |
| rockstar | I am confused too. WTF... | 18:50 |
| sinzui | rockstar: the difference between that template and the macro is that macro also guarantees not extra whitespace is generated | 18:51 |
| rockstar | sinzui, is that really a problem? | 18:51 |
| sinzui | We had s lot of mismatched icons during the 3.0 conversion because they were hard coded. | 18:52 |
| sinzui | rockstar: 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. | 18:53 |
| rockstar | sinzui, the formatter doesn't let me set the class of the edit icon, does it? | 18:54 |
| * rockstar doesn't see. | 18:54 | |
| === danilos is now known as danilo-afk | ||
| sinzui | rockstar: it sets the <a> link class, then created a <span class="invisible-link">link text</span> | 18:57 |
| sinzui | There is no icon, we use sprites | 18:57 |
| rockstar | sinzui, yes, but the class on the icon is important to the widget. | 18:57 |
| sinzui | okay now we are making progress | 18:57 |
| rockstar | So we didn't need the handcrafted link before, but we do now. | 18:57 |
| sinzui | Does beuno know that his maoist death march to sprites is being undermined by the Chang Kai Shecks' nationalist lazr.js force | 18:58 |
| sinzui | rockstar: I leave the link as it is since as you point out, it is required. | 18:59 |
| sinzui | rockstar: I think we are done | 19:01 |
| rockstar | sinzui, thanks. | 19:01 |
| sinzui | rockstar: r=me | 19:02 |
| rockstar | sinzui, I do think we need to re-consider that kind of markup that lazr-js widgets need. | 19:02 |
| sinzui | rockstar: 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 | 19:05 |
| leonardr | sinzui: can i jump the allenap-ful queue with a tiny branch that just upgrades some launchpad packages? | 19:08 |
| leonardr | https://code.edge.launchpad.net/~leonardr/launchpad/launchpadlib-devel/+merge/14705 | 19:08 |
| sinzui | leonardr: certainly with me. I am not an OCR | 19:08 |
| * sinzui looks | 19:08 | |
| leonardr | argh | 19:08 |
| leonardr | the topic scrolled off my screen and so "sinzui" was the first name i saw | 19:09 |
| sinzui | np | 19:09 |
| leonardr | but you might want to review that anyway since it fixes the problem you were complaining about | 19:09 |
| sinzui | indeed I know the issue | 19:09 |
| sinzui | I am just waiting for the diff to appear | 19:09 |
| === 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 | ||
| bac | sinzui: are you reviewing leonardr or do you want me to | 19:14 |
| sinzui | I will review it | 19:15 |
| bac | sinzui: we've spent more time discussing who gets to review it than it'll take. i'm glad to see it land! | 19:16 |
| sinzui | indeed! | 19:16 |
| sinzui | leonardr: land your branch | 19:16 |
| leonardr | ok | 19:20 |
| === matsubara is now known as matsubara-afk | ||
| sinzui | bac: are you still about for a review | 21:41 |
| bac | sinzui: we're on our way out the door. | 21:41 |
| sinzui | okay | 21:41 |
| bac | is it short? | 21:41 |
| sinzui | 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 | 21:42 |
| sinzui | bac I created series like lucid 10.04 and verify we sort it on the DSP page | 21:42 |
| bac | sinzui: i'll have a look now | 21:45 |
| thumper | sinzui: up to relook at the project page change? | 21:49 |
| thumper | sinzui: it has a test now :) | 21:49 |
| sinzui | sure | 21:49 |
| bac | sinzui: done. nice. | 21:52 |
| * bac EOD & off to dinner | 21:52 | |
| sinzui | fab: I can make some people happy | 21:53 |
| sinzui | thumper: 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. | 22:10 |
| sinzui | 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') | 22:11 |
| thumper | sinzui: 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 | 22:12 |
| thumper | sinzui: we could add yet another view that knows to go back to the main page | 22:12 |
| sinzui | thumper: not think, we must know. That view should return the user, that is part of the story | 22:12 |
| thumper | sinzui: I'm open to suggestions | 22:12 |
| sinzui | if you want to test state write a doctest or unittest | 22:13 |
| thumper | sinzui: so, what do you suggest for the story? | 22:13 |
| thumper | sinzui: write it as it happens | 22:13 |
| thumper | sinzui: or change what happens | 22:13 |
| sinzui | changing the order will make a simple narrative instead of something from a David Lynch movie | 22:14 |
| sinzui | owner seeing nothing, he uses the edit link, he submits the form, he sees it worked | 22:14 |
| sinzui | anon visits and sees the information too | 22:15 |
| sinzui | thumper: 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? | 22:20 |
| thumper | probably not | 22:21 |
| thumper | there isn't always a link | 22:21 |
| sinzui | that is disappointing | 22:21 |
| thumper | how do we handle sometimes menu items? | 22:21 |
| thumper | are you meaning on the menu for the branch? | 22:22 |
| sinzui | 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 | 22:23 |
| thumper | hmm | 22:23 |
| sinzui | thumper: I was hoping for a menu item so that you did not have to make the sprite | 22:24 |
| sinzui | thumper: I see lots of class="sprite info" | 22:24 |
| thumper | I can change it | 22:25 |
| === fjlacoste is now known as flacoste | ||
| sinzui | thumper: I think we are done here. fix the link and reorder the test. | 22:32 |
| thumper | sinzui: ok | 22:32 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!