[01:24] bac: can you please check the status of this MP https://code.edge.launchpad.net/~adiroiban/launchpad/bug-193750 ? [01:24] does it needs any action from me? [01:31] adiroiban: if you've made the changes requested by abel you just need to catch up with him and ask him to give it a once-over and land for you. sorry i don't have the time right now to look at it in detail. === 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 [01:32] bac: np :) just wanted to know what is the next step . thanks! [01:33] adiroiban: np. thanks for the fix and run down abel to get it landed. [07:52] Hi thumper: I can't merge your branch without conflicts? http://pastebin.ubuntu.com/337836/ [07:53] noodles775: I'll fix the conflicts [07:53] k [08:07] noodles775: pushed [08:07] ta. [08:07] noodles775: mechanical Y.get -> Y.one changed [08:07] hope everything still works [08:07] :) [09:06] Can somebody please review https://code.edge.launchpad.net/~wgrant/meta-lp-deps/lp-soyuz-deps/+merge/15836? [09:18] wgrant: allenap should be around soon (see https://dev.launchpad.net/ReviewerSchedule). [09:18] noodles775: Hello there :) === allenap changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [09:18] ah, timing :-) [09:19] wgrant: Hello, I'll take a look. [09:25] wgrant: r=me, but I'd like you to ask someone from soyuz to take a quick look too. Thanks :) [09:26] bigjools: Can you give https://code.edge.launchpad.net/~wgrant/meta-lp-deps/lp-soyuz-deps/+merge/15836 a glance, as discussed earlier? [09:26] allenap: Thanks. [09:26] wgrant: sure, but it won't be for ~3 hours [09:26] bigjools: OK. [09:49] allenap: Morning. Care to take a look at code / js for https://code.edge.launchpad.net/~gmb/launchpad/ajax-dupefinder-has-risen-from-the-grave? [09:50] gmb: I'd love to :) [09:50] Ta :) === 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 [10:14] gmb: I've merged Frankendupe into a recent devel and I get a syntax error in +filebug. [10:14] Poo [10:14] allenap: I'll look into that. [10:14] gmb: filebug-dupefinder.js line 260 [10:16] allenap: Ah, I see it. My bad; didn't lint after merging. I'll fix it presently. [10:17] allenap: Fixed and pushed [10:18] gmb: Cool. [10:19] Argh. Bloody DB patches. [10:28] allenap: could you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/bug-344054/+merge/15857 ? [10:30] adeuring: Sure. I'm doing gmb now, but you're next. === allenap changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: gmb || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [10:30] 0.o [10:30] allenap: thanks! [10:30] gmb: What does that mean? [10:31] allenap: It's the equivalent of a raised eyebrow or surprise or... something. Was responding to something that I think I've managed to do wrong... gimme a sec here... [10:32] gmb: Ah, okay. [10:32] Argh, looks like the last merge broke something. [10:36] Wow, heroic amount of suck from chromium this morning... [10:36] gmb: Shall I do Abel's while you're fixing that, or shall I just review what's there now? [10:36] allenap: Well, quick question: do you know what LPS is as opposed to using YUI? [10:36] Cos I think that's what's broken. [10:36] gmb: Ah, yes, instead of using new YUI(), just use LPS. [10:37] allenap: But then can I still use YUI.add() in filebug-dupefinder.js? [10:37] I'm seeing lots of "YUI is not defined" errors [10:38] gmb: Yeah, that should work. [10:38] gmb: How odd. [10:39] Hmm. [10:39] allenap: Okay, feel free to take a look at abel's. I'll get back to you shortly. [10:39] Argh. "LPS is not defined" Gaaaah. [10:41] gmb: Yeah. I didn't see anything on the list about this LPS change, and it's quite a big change, grumble, grumble :-/ [10:41] allenap: Yes. This is... suboptimal. [10:41] gmb: make clean, make should fix that I think. [10:42] noodles775: Ah, thanks. === allenap changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: adeuring || queue [gmb] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [10:43] noodles775: Incidentally, did you see my reply to your UI review? [10:44] gmb: I did get to skim through it yesterday, I'll take a better look now. Sorry :/ [10:44] noodles775: No worries. I'd like to get this landed today though, even if I have to do a lot of small branches to get it prettified for release. It's a big delta to be carting around between cycles. [10:45] gmb: one thought I did have (not ui, but related to the code), there should be a better way to avoid the 'hitting enter causing the search to be re-run'. [10:45] You should be able to catch the onsubmit event and disable the default behaviour? [10:46] er, that should be the other way around... not avoid, but ensure. [10:46] noodles775: ISTR trying that several cycles and it failing miserably, which is why I went straight for the keypress. But you're right; I'll give it another shot. [10:46] *cycles ago [10:47] * noodles775 remerges the branch. [10:48] adeuring: My gut feeling is that regular_attachments and patches should be view properties rather than model properties. Did you consider that? [10:49] allenap: right, good idea. didn't think about that [10:52] allenap: YUI() changed to LPS... the latest revision should work fine now. [10:55] adeuring: My feeling is that they should not be exported in any case. If you're a consumer of the API, I think it's better to get the list of all attachments and split it up from there; the criteria for a patch are unambiguous and easy to test. Exporting two (well, three) attachment fields, we also encourage people to make multiple API calls. [10:55] gmb: Cool. I should have said that it was working for me earlier, and very nice it is too :) [10:55] allenap: yeah; I'll use browser methods [10:56] adeuring: Thanks, and sorry for being a downer. [10:56] allenap: np ;) [10:56] allenap: Well, I needed to update YUI -> LPS anyway, so no worries :) [10:59] noodles775: You're right; I've no idea what Iwas doing wrong before. I've fixed that. === allenap changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: gmb || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [11:00] Great gmb ! [11:01] allenap: Please pull the latest revision again; I've stopped using the button's onClick event and started catching the form's onSubmit instead as noodles775 suggested. [11:01] gmb: Cool, will do. [11:06] gmb: ui=me, thanks for all the improvements! [11:07] noodles775: Thanks! [11:08] hi gmb [11:09] gmb: i looked at your branch yesterday but you'd already left. i couldnt' get anything to work. perhaps that's what youv'e been discussing here. [11:09] Hi bac. [11:09] none of the advertised JS stuff did anything [11:09] twisties didn't twist [11:09] bac: Possibly. There was a syntax error in the JS after a merge. [11:09] Apologies for that; I didn't lint after merging. [11:09] gmb: ok. sorry i had to punt on it [11:11] No worries. [11:11] bac: It seems that there was a syntax error in lp:~jpds/launchpad/fix_196173 that I didn't catch yesterday, I've pushed a fixed revision. [11:12] jpds: ok, i haven't looked at the results from the tests yet [11:16] allenap: http://paste.ubuntu.com/337919/ [11:16] bac: I've made sure that all registry tests pass on the branch, not just the distributionmirror ones. [11:16] adeuring: Cool, I'll look at that in a bit. [11:16] jpds: ok. i'll get your new version and submit it shortly [12:02] jpds: your branch was submitted to ec2 about 30 minutes ago [12:02] bac: Neat, thanks. === matsubara-afk is now known as matsubara [12:09] * gmb -> lunch === henninge is now known as henninge-lunch === 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 [12:35] gmb: I'm still doing your review. I'll finish it when I'm back, but I've done the first 250 lines here if you want to take a look: http://pastebin.ubuntu.com/337964/ === henninge-lunch is now known as henninge [13:04] allenap: Awesome, thanks [13:07] henninge: hi. is there anything I need to do for https://code.edge.launchpad.net/~adiroiban/launchpad/bug-193750/+merge/15292 ? [13:11] adiroiban: was it not merged yet? [13:12] henninge: the status is still Aproved [13:12] adiroiban: didn't you get emails like "success" for your branches? [13:13] adiroiban: also you could just update your devel and diff your branch against it to see if it is merged. [13:14] adiroiban: because I see that it was merged. Dunno why the MP did not pick up on it. [13:14] it may have to do with the fact that I had to re-submit it directly to PQM because of a testfix mode bounce. [13:15] adiroiban: can you change the status of the MP to merged, or can only I do that? [13:45] allenap: I've taken care of those items; If you post that part of your reviewto the MP I'll reply to it. [14:27] noodles775, beuno: could one of you please ui-review this branch. https://code.edge.launchpad.net/~adeuring/launchpad/bug-344054/+merge/15857 ? (now asked in the right channel...) [14:27] adeuring: I can, but it's worth checking with sinzue or other UI reviewers first (so we all get to learn/practise). [14:27] s/sinzue/sinzui [14:28] ah, right, so, sinzui: ^^^ [14:28] adeuring: I can do that in 15 minutes [14:28] sinzui: thanks! [14:42] hello allenap, could you please take a look at https://code.edge.launchpad.net/~al-maisan/launchpad/ejdt-484819/+merge/15872 when you get to it? === al-maisan changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: gmb || queue [al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [14:48] al-maisan: Sure, it will be my pleasure :) [14:48] allenap: thanking you. === henninge is now known as henninge-bbl === EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: allenap, Edwin || reviewing: gmb || queue [al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [14:58] allenap: do you have any reviews waiting that you would like me to take? [14:58] EdwinGrubbs: al-maisan has a branch that I haven't looked at yet. I'm sure he'd appreciate it. [14:58] sure [14:59] EdwinGrubbs: it's here: https://code.edge.launchpad.net/~al-maisan/launchpad/ejdt-484819/+merge/15872 [14:59] Thank you very much for looking at it! === salgado is now known as salgado-lunch [15:27] allenap: How's that branch looking? [15:28] gmb: Sorry, I've been distracted by phoning Philips about the very large number of expired light bulbs I have. [15:28] gmb: I'm back on it now. [15:29] allenap: Okay. Can you post the part of the review you did earlier to the mp so I can reply to it? Would help me keep track a bit, I think. [15:29] gmb: Sure, good idea! [15:35] allenap: So, that's *just* for the JS, right? Just being clear since there are two reviews requested there. === beuno is now known as beuno-lunch [15:36] gmb: Yes, I've just added another review for the first part of the code. [15:36] Cool [15:47] sinzui: did ou forget the ui review ;)?look at the [15:47] al-maisan: in Build.createBuildQueueEntry() will it always create a BuildQueue with job_type=BINARYBUILD, or is this just temporary since this is an intermediate branch? [15:48] adeuring: I am playing with it now [15:48] sinzui: ah, OK, thanks [15:48] adeuring: I was distracted for 30 minutes [15:48] sinzui: no problem [15:49] EdwinGrubbs: it will always do it .. other job type classes will have their own equivalents of createBuildQueueEntry() that set the job type and the estimated job duration as is appropriate to them [15:50] other job type classes are forthcoming and will be something along the lines of "Create source package from recipe" [15:50] .. or translations import [15:50] al-maisan: should the Build class be renamed BinaryBuild then? [15:50] EdwinGrubbs: yes, eventually it will be. [15:51] likewise for the db table that underpins it === salgado-lunch is now known as salgado [15:57] gmb: Ah, it seems the merge proposal system doesn't recognise my approval for both js and code. [15:58] allenap: Hah. [15:58] allenap: Well, thanks anyway :). I'll make the necessary changes and land it posthaste. [15:58] EdwinGrubbs: Could you add a "code" approved review to https://code.edge.launchpad.net/~gmb/launchpad/ajax-dupefinder-has-risen-from-the-grave/+merge/15599, saying that I reviewed it. It doesn't seem to recognise my approval for both js and code. [16:00] allenap: I can do that, but I usually just see reviewers enter "ui code" when they are reviewing both. === matsubara is now known as matsubara-lunch [16:01] EdwinGrubbs: I reviewed js before I did code, so I wasn't able to do that. Sounds like I need to file a bug. [16:01] allenap: You can submit another review, and it will overwrite the review type at the top of the page. [16:02] allenap: Does the frontpage +filebug form still exist? I can't remember how to get to it. [16:03] gmb: I don't think so. [16:03] allenap: Cool. [16:05] allenap: do you still want me to add a separate review? [16:08] abel: I replied with a suggestion. We can talk about it if you have misgivings [16:09] allenap: Just do a quick review with the message type to code js [16:09] It will overwrite your old one. [16:09] I'll delete the second request. [16:09] Except that I can. [16:09] 't [16:09] Gah. [16:10] allenap: All that I care about is that you mark the MP as approved ;) [16:10] gmb: Done. [16:10] Thanks. [16:20] al-maisan: is the _delayCausedByPendingJobsAhead() called very often? I'm wondering if it is worthwhile to have it perform a more efficient query. It doesn't look like pendingJobsQuery() is called anywhere else. Am I missing something? [16:21] EdwinGrubbs: I submitted js and code reviews separately at first. I just edited my review at the top to be "code js", but it still shows a review request for ~launchpad. As long as it doesn't show as needing a review in https://code.edge.launchpad.net/launchpad-project/+activereviews then it's fine. Thanks. [16:24] EdwinGrubbs: I think there is a bug there; your review is showing as Pending. [16:25] allenap: do you mean my review of muharem's branch? I just claimed the review five seconds ago. [16:25] EdwinGrubbs: Did you also claim the review of gmb's branch? https://code.edge.launchpad.net/~gmb/launchpad/ajax-dupefinder-has-risen-from-the-grave/+merge/15599 [16:26] allenap: Does that review matter any more? The branch should be merged before EoD. [16:26] gmb: No, the review itself doesn't; it's all good. But I'm interested in figuring out if there's a bug to report. [16:26] allenap: oh, yes, I did that, but when I went to +activereviews, I saw that it was in the right place, so I didn't go back to the mp. I'll add my fake review to avoid any more confusion. [16:27] EdwinGrubbs: Okay, sorry for the noise. === abentley1 is now known as abentley [16:34] EdwinGrubbs: _delayCausedByPendingJobsAhead() will be called for individual, pending build jobs [16:35] the idea re. pendingJobsQuery() is that each BuildFarmJob class implements it [16:35] then the "central" estimation logic can collect the SELECT clauses from each of them and hit the db with a single SELECT/UNION query [16:36] al-maisan: so this will just be called in a cronjob, it won't be called each time a certain page is loaded? [16:36] EdwinGrubbs: the latter [16:37] EdwinGrubbs: here's an example: https://edge.launchpad.net/ubuntu/+source/darkroom/1.5.0~svn1037635-0ubuntu2/+build/1387612 [16:38] I was trying to put in place a general job dispatch time mechanism/scheme in which all build farm job types can participate .. === beuno-lunch is now known as beuno [16:39] .. provided they implement 3 callback functions [16:39] 1: pendingJobsQuery [16:39] 2: processor [16:39] 3: virtualized [16:39] al-maisan: Are you worried about this slowing down the page when their are a lot of pending build requests? [16:40] EdwinGrubbs: maybe .. 2 other build farm job types will be added in the near future [16:41] i.e. what is 1 query now will become "SELECT .. UNION SELECT .. UNION SELECT .." [16:42] EdwinGrubbs: I am obviously open to suggestions how to make the estimation of job dispatch times perform better [16:45] al-maisan: I'm wondering if it would be feasible to pass in the id, virtualized and processor.id values of the current build into pendingJobsQuery so that the query can sum up the durations for you. Of course, I don't know if the other unimplemented job types would complicate this. [16:47] beuno, noodles775: could one of you please have a ui-look at this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-344054/+merge/15857 ? [16:48] adeuring: I can do it first thing tomorrow if beuno doesn't beat me to it. [16:48] EdwinGrubbs: I am not sure this will work because 1 - the final order of the jobs is only established by an ORDER BY clause on the entire "SELECT .. UNION SELECT .. UNION SELECT .." query [16:48] noodles775: thanks! [16:48] adeuring, noodles775, I can take care of it if it has a screenshot [16:48] beuno: just a second... [16:49] EdwinGrubbs: 2 - even though a job is ahead in the queue wrt our job of interest, it may not compete with the latter for builders i.e. it should be ignored for the purpose of estimating the dispatch delay [16:51] EdwinGrubbs: what I could probably do is: pump the "SELECT .. UNION SELECT .. UNION SELECT .." result set into a temporary table and continue to query that as opposed to performing calculations in the python domain [16:51] EdwinGrubbs: However, I'd like to do that only if and when we run into performance issues [16:52] al-maisan: Regarding #1, why does the order of the results matter? Your for-loop over the results doesn't seem to depend on the order, since the pendingJobsQuery already filters out jobs with a lower score. [16:53] EdwinGrubbs: good question, let me think about that for a moment [16:54] beuno: http://people.canonical.com/~adeuring/attachments-patches.png (new portlet "patches"; changes link text "add attachment or patch" at the bottom [16:55] adeuring, I think sinzui did a great review, I'll approve as well [16:55] beuno: great, thanks! [16:57] al-maisan: Regarding #2, the query can ignore the right rows if you pass in the id, processor.id and virtualized parameters. For example: SELECT SUM(estimated_duration) FROM .. WHERE .. AND BuildQueue.id != my_id AND (my_processor IS NULL OR Build.processor IS NULL OR (Archive.require_virtualized = my_virtualized AND Build.processor = my_processor) [17:01] EdwinGrubbs: these are very good ideas! Let me think about them a bit more and I'll come back to you. Thank you drilling down into these queries! [17:01] *for drilling.. === allenap changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: al-maisan || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === matsubara-lunch is now known as matsubara [17:44] allenap: lol, and thanks for the review [17:45] bigjools: Welcome :) [17:45] allenap: it's a shame optparse can't deal with non-option args (or can it?) === deryck is now known as deryck[lunch] === henninge-bbl is now known as henninge [17:47] bigjools: You can use a callback to absorb more than one argument, but I think it still needs a named arg to trigger it. [18:50] EdwinGrubbs: thank you again for your thought-provoking questions, I am retracting the MP and will rework the branch. [18:53] ok === deryck[lunch] is now known as deryck === EdwinGrubbs is now known as Edwin-lunch [20:00] salgado, bac: https://code.edge.launchpad.net/~jpds/launchpad/fix_50580/+merge/15892 === henninge is now known as henninge-bbl [20:18] bigjools: Are you going to have time to look at the lp-soyuz-deps branch today? [20:24] wgrant: I was going to badger him about it earlier but decided against it, so bigjools, consider yourself retrospectively poked ;) [20:27] I'd reeeally like to avoid this slipping to week 4, as then it's bound to slip to January. === salgado is now known as salgado-afk === henninge-bbl is now known as henninge [21:10] wgrant: it looks ok to me === EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [21:30] bigjools: Thanks. [21:30] * wgrant merges. [21:30] Actually, not sure if I'm allowed to. === matsubara is now known as matsubara-afk [22:27] EdwinGrubbs: hey, I have a very trivial fix to bug 494055 in http://bazaar.launchpad.net/~lfaraone/launchpad/bugfix494055/revision/10008. Per PatchSubmission, I'm poking you. [22:27] Bug #494055: "Report bug upstream" link uses literal "&" in URL. [22:29] lfaraone: I can look at that. [22:29] EdwinGrubbs: awesome, thanks. === sinzui changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [22:39] EdwinGrubbs: do you have time to review https://code.launchpad.net/~sinzui/launchpad/false-packages/+merge/15901 [22:40] EdwinGrubbs: do I need to propose the branch for merging or something similar? [22:41] lfaraone: yes, you need to do that. Sorry, I hadn't looked at your link yet. If it is a simple fix, you don't have to worry about a big description. [22:41] sinzui: sure [22:45] EdwinGrubbs: set to merge into lp:launchpad/devel, ~edwin-grubbs as reviewer. [23:19] lfaraone: the change looks good, but you will definitely need to fix the test lib/lp/bugs/doc/bugtracker.txt. You can run that test with the command: ./bin/test -vvt bugtracker.txt [23:20] lfaraone: it would also be a good idea to run all the bug tests with the command: ./bin/test -vvt lp/bugs [23:21] lfaraone: I can help you get this merged after you make the changes. [23:25] EdwinGrubbs: oh, right.