=== noodles775 changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [sinzui, noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === salgado is now known as salgado-afk [10:17] noodles775: Hi! could you do a ui review for bug 425583? [10:17] Bug #425583: Improve language set index page [10:18] noodles775: the bug has a mockup attached to it, there is no branch or mp yet. [10:18] henninge: sure. [10:18] Seeing how this release week is a quiet one for you ... ;-) [10:19] henninge: I don't suppose I can do a swap with you? [10:19] https://code.edge.launchpad.net/~michael.nelson/launchpad/487009-db-more-soyuz-extraction/+merge/15533 [10:19] noodles775: sure you can ;) [10:20] Thanks! [10:53] henninge: ok, done. It looks great! I added a few thoughts that came up as I thought about interacting with it. [10:54] noodles775: cool, I'll look at it in a minute [10:54] noodles775: about your branch [10:55] without wanting to check each line, I trust that the code moved to dispatchBuildToSlave, verifyBuildRequest, _cachePrivateSourceOnSlave, _extraBuildArgs was copied verbatim? [10:55] Almost verbatim, I had to check all references to self, so that self.url would become self._builder.url. [11:00] noodles775: and you had to add a call to _extraBuildArgs [11:02] henninge: The extra build arguments used to be created in startBuild(), so yes, I copied that code out into BinaryPackageBuildBehavior._extraBuildArgs() and called it from the specific dispatchBuildToSlave instead. [11:04] noodles775: I am trying to decide if this is a pure refactoring branch or if it adds functionality that would need tests. [11:04] noodles775: what about the new code for IdleBuildBehavior? === matsubara-afk is now known as matsubara [11:06] henninge: yep, as mentioned in the MP, I'm going to add some documentation for the build behavior infrastructure in the next branch, but I've certainly tried to ensure that I'm not adding any new functionality to IBuilder in this branch, just pure refactoring. [11:06] noodles775: also the return value of "status" is not demonstrated/tested. [11:07] noodles775: ah, I see === salgado-afk is now known as salgado [11:10] henninge: so, the current behavior of the status property is tested in doc/builder.txt (not exhaustively), and I've tried to ensure that I haven't modified its behaviour. [11:10] But without a doubt, it could be better tested. [11:17] noodles775: oh, I missed that that is moved code, too. Found it now at the end of the patch ;) [11:18] Ah, great :D [11:18] Note, in that case it's split code, rather than moved. [11:18] noodles775: let me just verify that the test is really passing and then you're good to go [11:18] Ta! [11:19] yes, the 'Idle' went to the Idle...Behavior [11:27] noodles775: done r=me [11:27] Thanks henninge ! [11:28] allenap, do you plan to write a windmill test for your fix to bug 489342? [11:28] Bug #489342: "'display_name' is undefined" JavaScript error on development [11:28] salgado: Maybe, but I don't really know where to start right now, and I want to get it landed ;) [11:29] salgado: I can promise to write a test and do manual QA for the release. [11:29] If that's okay with you. [11:29] allenap, that's ok, yes, but can you file a bug for the missing test so that we don't forget about it? === noodles775 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 [11:30] salgado: Certainly. Thanks. [11:30] salgado: I'll go land now. [11:30] allenap, please land on db-devel [11:30] salgado: Sure. [11:30] I need to get devel closed [11:32] salgado: Gah! I just submitted it and it's gone in for devel. [11:33] salgado: I can resubmit to db-devel if you want to close devel. [11:33] allenap, delete the branch from LP [11:34] salgado: Okay. [11:34] or push --overwrite a 'devel' branch on top of it [11:34] and then resubmit [11:35] seems to have worked [11:40] noodles775: thanks for your comment on the languages page. I just replied to it. === henninge is now known as henninge-afk === 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 === allenap changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === henninge-afk is now known as henninge [12:37] allenap, stub's got a RC branch up for review, can you take it? [12:37] https://code.edge.launchpad.net/~stub/launchpad/opstats-nodb/+merge/15543 [12:49] allenap, ping, would you be able to review a brief set of fixes to the JS unit test suite? https://code.launchpad.net/~mars/launchpad/fix-js-unittests/+merge/15546 === henninge is now known as henninge-afk [13:34] salgado: Sure [13:34] mars: Sure. [13:46] salgado, stub: r=me === allenap changed the topic of #launchpad-reviews to: on-call: allenap || reviewing: sinzui || queue [mars] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === henninge-afk is now known as henninge === matsubara is now known as matsubara-lunch === EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: allenap, Edwin || reviewing: sinzui || queue [mars] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [15:29] allenap: would you like me to take mars' branch? [15:29] EdwinGrubbs: I did it already, sorry. === 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 [15:30] you shouldn't be sorry. I should be thanking you for saving me work. [15:30] lol [15:30] allenap, thanks for the review [15:30] Heh :) [15:31] mars: Welcome :) === beuno is now known as beuno-lunch === matsubara-lunch is now known as matsubara [16:47] allenap or EdwinGrubbs -- I have a simple js unit test fix. 30 line diff. === deryck changed the topic of #launchpad-reviews to: on-call: allenap, Edwin || reviewing: - || queue [deryck] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [16:47] https://code.edge.launchpad.net/~deryck/launchpad/fix-me-too-unit-test-491441/+merge/15561 [16:47] deryck: I can take it. [16:48] EdwinGrubbs, cool, thanks. === allenap changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [deryck] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [16:49] deryck: I assume this is not an RC candidate. [16:49] EdwinGrubbs, no, it's not. [16:49] Just want it ready when pqm opens is all. [16:49] simple fix, so catch it while I can. === beuno-lunch is now known as beuno === 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 [19:42] EdwinGrubbs: you have time for a testfix/reroll candidate? https://code.edge.launchpad.net/~gary/launchpad/opstats-nodb/+merge/15569 [19:44] gary_poster: sure, looking now [19:47] gary_poster: r=me [19:50] thank you === salgado is now known as salgado-afk === matsubara is now known as matsubara-afk === jamalta_ is now known as jamalta