=== 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
=== salgado-afk is now known as salgado
[08:06] <henninge> noodles775: Guten Morgen! ;)
[08:06] <noodles775> Hi hennige!
=== 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
[08:07] <noodles775> :-D
[08:07] <henninge> noodles775: ah, you are the ocr! :)
[08:07] <noodles775> Yeah, myself and al-maisan are around today.
[08:08]  * al-maisan will be around after preparing his cappuccino <wink> :)
[08:08] <henninge> noodles775: cool, let me prepare an mp for a final ui review. I'd like to have your input on the languages page.
[08:08] <noodles775> Wow - that was fast?
[08:14] <henninge> noodles775: well, I did not go for using and improving the lazr-js widget, I am sorry.
[08:14] <henninge> noodles775: but I quite like what's come out of it and I only had yesterday to do it. ;)
[08:15] <noodles775> henninge: no, I didn't expect you to do that as part of the initial branch (it's not necessary for the improved behavior, it'd just be a nice bling later).
[08:15] <henninge> noodles775: It *does* have the client-side filter, just not as-you-type. You need to click.
[08:15] <henninge> oh, and windmill test ist still missing ...
[08:16] <noodles775> Aha.
[08:34] <henninge> noodles775: ok, mp is done.
[08:34] <henninge> noodles775: I'd ask you to please branch it and have a look at the page.
[08:35] <noodles775> Great, yep, I always do :-)
=== noodles775 changed the topic of #launchpad-reviews to: on-call: noodles775 || reviewing: - || queue [henninge/ui,henninge/code] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[08:35] <henninge> noodles775: also, I am not sure if I am writing good js/yui code.
[08:36] <noodles775> OK, I'll check it out too.
[08:43] <henninge> noodles775: fyi, just pushed a new version.
[08:45] <noodles775> henninge: is this dependent on other changes currently in db-devel only? Or why are you targeting db-devel?
[08:46] <henninge> noodles775: well, I just don't want to get caught in yui3final changes by working against the latest version.
[08:47] <henninge> noodles775: Also, after the roll-out (when this lands) db-devel will have become the new devel. So eventually it will be merged into devel.
[08:47] <noodles775> ah, I thought the yui3final stuff was to be merged back into devel by now, maybe it wasn't (or I misunderstood). But yes, +1 for landing this on devel (so it hits edge) after the roll-out.
=== al-maisan changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: -, - || queue [henninge/ui,henninge/code] || 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: noodles775, al-maisan || reviewing: henninge/ui, - || queue [henninge/code] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[09:14] <noodles775> henninge: wow - I didn't realise that you had used the picker for the filtering! It should be really easy to update what you've done to filter on keypress.
[09:15] <henninge> noodles775: beg your pardon?
[09:15] <noodles775> henninge: I didn't realise that you'd actually done the filtering via JS - I was expecting the normal GET to +languages?blah
[09:15] <noodles775> It's great!
[09:16] <henninge> noodles775: Ah! Yes, that was fun I had after hours yesterday ... ;)
[09:19] <henninge> noodles775: oh, I just read about the "unseen" css class.
[09:25] <henninge> windmill doc is out-of-date ... :(
[09:26] <noodles775> which one?
[09:26] <noodles775> or you mean the actual documentation?
[09:26] <henninge> https://dev.launchpad.net/Windmill
[09:26] <henninge> I read about the "unseen" class there but it does not seem to exist. But I am still checking.
[09:26] <noodles775> BTW: I think you'd be best creating JS unit-tests for your new module...
[09:27] <noodles775> (you'd still need one very basic windmill test though, hrm, so maybe it's easier just to do the windmill test for the moment).
[09:28] <noodles775> henninge: also, all_languages should be a module-level var I think? (ie. so it's not evaluated each time hide_and_show is called)?
[09:29] <henninge> noodles775: yes, sounds reasonable.
[09:29] <henninge> noodles775: the "unseen class does exist in style.css". Is that not generally available?
[09:30] <henninge> I could just move it to style-3.0.css
[09:34] <noodles775> I'd be careful - Curtis is landing a branch very soon that removes a lot of the old styles... so yes, perhaps move it but be prepared for conflicts.
[09:36] <noodles775> bbiab - OTP
[09:44]  * henninge has to go afk for 15 min
=== henninge is now known as henninge-afk
=== henninge-afk is now known as henninge
[10:13] <noodles775> al-maisan: are you ok to do the code review for henninge's branch? (beuno has a rule about not doing code reviews *and* ui reviews for the same branch).
[10:14] <noodles775> (I think to do with making UI assumptions based on code and vice-versa, but if it's a real hassle for you at the moment, I can do it).
=== noodles775 changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: - , - || queue [henninge/code] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[10:15] <noodles775> al-maisan: actually, you'll probably want to wait until the ui review is finished anyway (as henninge might have a few more changes from the UI review.)
[10:15] <henninge> noodles775, al-maisan: yes, and the windmill test is still missing.
[10:16] <henninge> noodles775: I am reading the review atm
[10:16] <noodles775> k
[10:28] <al-maisan> ok, thanks for letting me know.
[10:43] <gmb> noodles775: Can I get a preliminary UI review for https://code.edge.launchpad.net/~gmb/launchpad/ajax-dupefinder-has-risen-from-the-grave/+merge/15599 please? It's pretty rough around the edges but I'd rather get some feedback before starting to polish it.
=== jelmer_ is now known as jelmer
[10:43] <noodles775> gmb: sure!
[10:43] <gmb> Thanks
=== noodles775 changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: gmb-ui , - || queue [henninge/code] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[10:53] <noodles775> gmb: er, wow.
[10:53] <gmb> noodles775: That's not a good word.
[10:53] <noodles775> Well, I was just amazed - it's super responsive :-)
[10:54] <noodles775> and the animation looks really nice.
[10:54] <gmb> noodles775: Heh - I suspect it won't be quite so responsive with a metric assload of bugs.
[10:55] <noodles775> Ah, actually, I think the animation is unintended (yep, I just read your MP)
[10:55] <noodles775> Well, still faster without a full page load hopefully :-)
[10:57] <gmb> Yeah :)
[11:23] <noodles775> gmb: sent.
[11:23] <gmb> Cool
[11:27] <gmb> noodles775: Thanks; plenty for me to chew on. I'll let you know how I get on :)
[11:27] <noodles775> Great, thanks.
=== matsubara-afk is now known as matsubara
=== mrevell is now known as mrevell-lunch
=== noodles775 changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: - , - || queue [henninge/code] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[12:30] <noodles775> al-maisan: ok, we've finished the ui review for henninge's branch :)
[12:30] <henninge> noodles775: cool, thank you!
[12:30] <noodles775> np!
[12:30] <al-maisan> noodles775: thanks for letting me know, I can go ahead and review the other branch now?
[12:30] <henninge> al-maisan: please wait until I commit the windmill test. Getting there ...
[12:31] <al-maisan> henninge: fine .. I need to break for lunch anyway :)
[12:31] <noodles775> al-maisan: it's the same branch, but yep (what he said).
=== mrevell-lunch is now known as mrevell
=== danilo_ is now known as danilos
=== henninge_ is now known as henninge
[14:02] <henninge> al-maisan: the branch is ready for review now. Thanks for waiting!
[14:03] <henninge> noodles775: can you please enter your vote on the mp page? Thanks. ;)
[14:03] <henninge> al-maisan: I will relocate now, back in 30 min max.
[14:03] <al-maisan> henninge: please point me to your branch first
[14:03] <henninge> :)
[14:03] <al-maisan> henninge: where is it?
[14:03] <henninge> al-maisan: https://code.edge.launchpad.net/~henninge/launchpad/bug-425583-languages/+merge/15594
[14:03] <al-maisan> henninge: thanks!
[14:10] <henninge> noodles775: sorry, didn't see your vote. Forgot to reload the mp page ;-)
=== al-maisan changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: - , henninge/code || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[15:06] <al-maisan> hello henninge, could you please take a look at http://pastebin.ubuntu.com/333878/ ?
[15:07] <henninge> al-maisan: looking
[15:07] <henninge> al-maisan: ah yes, I just noticed myself and fixed it.
[15:07] <al-maisan> henninge: great!
[15:07] <al-maisan> henninge: did you push the changes?
[15:08] <henninge> pushing them as we "speak" ;-)
[15:08] <henninge> done
[15:08] <al-maisan> thanks
[15:08] <henninge> sorry about that ...
[15:10] <al-maisan> henninge: I also get http://pastebin.ubuntu.com/333882/
=== salgado is now known as salgado-lunch
[15:10] <henninge> al-maisan: haven't seen that before ...
[15:11] <al-maisan> hmm .. that's funny, I get it when running "bin/test --layer=TranslationsWindmillLayer --test=filter_languages"
[15:12] <al-maisan> henninge: it may be a problem with my local system though .. as long as you land this through ec2 I am happy.
[15:13] <al-maisan> henninge: there's a small typo in lib/lp/translations/browser/tests/language-views.txt
[15:13] <henninge> al-maisan: I am just doing that myself again and don't get this error.
[15:13] <al-maisan> henninge: see my remark about landing via ec2 above
[15:14] <al-maisan> re. the type please s/For s user/For a user/
[15:14] <henninge> al-maisan: does ec2 run windmill tests?
[15:14] <henninge> al-maisan: oh, thanks :)
[15:14] <al-maisan> err .. good question.
[15:14] <al-maisan> BjornT_just sent out an email about this I believe
[15:15] <al-maisan> henninge: so, will this be submitted to db-devel or the jscheck builder?
[15:15] <henninge> al-maisan: I have never submitted to the jscheck builder ... 
[15:15] <henninge> am I supposed to do that?
[15:15] <al-maisan> From BjornT's email:
[15:15] <al-maisan> next week I'd like us to get rid of the jscheck buildbot builder, and
[15:15] <al-maisan> instead run the Windmill tests in the normal devel and db-devel (and
[15:15] <al-maisan> later also production-devel) builders.
[15:16] <henninge> al-maisan: ah
[15:16] <al-maisan> so, no windmill tests in the devel builder at present
[15:16] <BjornT_> henninge: no, ec2 doesn't run windmill tests. i tried running them, but it can run them. the images need to be updated
[15:16] <al-maisan> s/builder/buildbot/
[15:16]  * al-maisan has been dealing too much with Soyuz builders :P
[15:17] <henninge> al-maisan: I had planned to land this next week on staging. by then, db-devel will have become devel.
[15:17] <henninge> s/staging/devel/
[15:17] <al-maisan> I see
[15:17] <al-maisan> henninge: this is a good branch, very nice work, r=me
[15:18] <henninge> al-maisan: thank you very much!
[15:18] <al-maisan> you are welcome 
=== al-maisan changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: - , - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[15:49] <deryck> noodles775, or al-maisan -- I have a 12 line diff for review, to fix a broken js unit test.
=== deryck changed the topic of #launchpad-reviews to: on-call: noodles775, al-maisan || reviewing: - , - || queue [deryck] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[15:49] <deryck> https://code.edge.launchpad.net/~deryck/launchpad/subscriber-unit-test-fix-491453/+merge/15609
[15:49]  * noodles775 looks
[15:49] <al-maisan> noodles775: thanks.
[15:50] <noodles775> deryck: r=me
[15:50] <deryck> noodles775, thanks
[15:54] <EdwinGrubbs> noodles775, al-maisan: can one of you review my tiny branch? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-230801-renewing-membership/+merge/15610
=== beuno is now known as beuno-lunch
[15:54] <noodles775> EdwinGrubbs: sure!
[15:54]  * noodles775 is loving the 4 or 5 liners :)
[15:56] <noodles775> EdwinGrubbs: is this targeted to db-devel for a reason? (or are you planning on landing it on devel?)
[15:58] <EdwinGrubbs> noodles775: I'm planning on landing it on devel after pqm opens, and I wanted to prevent merge conflicts later, since devel still didn't have some of the revisions when I started the branch.
[15:58] <noodles775> k
[16:03]  * al-maisan bows out
=== al-maisan changed the topic of #launchpad-reviews to: on-call: noodles775 || reviewing: -  || queue [deryck] || 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: noodles775, rockstar || reviewing: -  || queue [deryck] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-lunch
[16:14] <bac> hi gary.  could i interest you in a short lazr.lifecycle review?
[16:14] <bac> gary_poster: ^^
[16:18] <gary_poster> bacsure
[16:18] <gary_poster> bac: sure
[16:18] <bac> gary_poster: https://code.edge.launchpad.net/~bac/lazr.lifecycle/snapshot/+merge/15612
[16:18] <bac> thanks
=== deryck is now known as deryck[lunch]
=== salgado-lunch is now known as salgado
[16:26] <gary_poster> bac: I like it.  two thoughts, one trivial, the other speculative.
[16:26] <gary_poster> 1) this is a new feature; therefore, I'd recommend releasing this as 1.1 not 1.0.1
[16:26] <noodles775> All yours rockstar :-)
=== noodles775 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
[16:27] <rockstar> noodles775, cheers!
[16:28] <gary_poster> 2) Did you consider adding sugar so that the documented mechanism is something like ``ignore_me = do_not_snapshot(Attribute('ignored attribute'))`` (or doNotSnapshot, whatever convention lazr.lifecycle is following now; probably camelCase).
[16:28] <gary_poster> s/./?/
[16:29] <gary_poster> in an explicit, non-vim kind of interpretation of the substitution :-)
[16:29] <bac> gary_poster: 1) ok and 2) that's a good idea.  it is rather unwieldy now
[16:30] <gary_poster> bac: ok cool.  You could keep the same mechanism, so it really is just sugar
[16:30] <gary_poster> bac: ok, do you want me to approve now, trusting the change, or look at the revision?  Up to you.
[16:30] <bac> gary_poster: i'll run the sugary fix past you
[16:31] <gary_poster> bac: ok cool.  ping me when you are ready
[16:45] <bac> gary_poster: something like: http://pastebin.ubuntu.com/333934/
[16:45] <gary_poster> looking
[16:46] <gary_poster> bac, I like that.  Do you like that?
[16:46] <bac> gary_poster: sure, with the addition of a doc string
[16:46] <gary_poster> bac: +1
[16:51] <gary_poster> bac, I approved, also with a request to update the CHANGES file (or whatever it is called)
[16:53] <gary_poster> oops
=== matsubara-lunch is now known as matsubara
=== deryck[lunch] is now known as deryck
=== beuno-lunch is now known as beuno
[18:21] <beuno> if wants free karma, I have a branch that just adds an SVG for merging: https://code.edge.launchpad.net/~beuno/launchpad/ppa-icon-svg/+merge/15620
[18:59] <sinzui> karma?
[19:00] <sinzui> beuno: rs=me
[19:01] <beuno> sinzui, どうもありがとう
[19:01] <sinzui> mr. roboto
[19:02] <beuno> :)
=== bigjools-afk is now known as bigjools
=== salgado is now known as salgado-afk
=== EdwinGrubbs is now known as Edwin-lunch
[20:14] <bac> hey rockstar you still have another review in you?
[20:23] <rockstar> bac, yup
[20:24] <bac> rockstar: thanks - https://code.edge.launchpad.net/~bac/launchpad/bug-488762-snapshot/+merge/15628
=== matsubara is now known as matsubara-afk
[20:25] <bac> rockstar: in case you're interested, it uses this lazr.lifecycle change: https://code.edge.launchpad.net/~bac/lazr.lifecycle/snapshot/+merge/15612
[21:13] <rockstar> bac, in lib/canonical/launchpad/webapp/snapshot.py you have a # DO NOT CHECK IN
[21:13] <rockstar> I'm not sure which one you're not supposed to check in.
[21:14] <bac> rockstar: well, i'm smart enough to put in traps
[21:14] <bac> rockstar: thanks for spotting it
[21:14] <bac> i thought i'd reverted that file...
[21:16] <rockstar> bac, why did you delete the branches property in lib/lp/registry/model/product.py
[21:17] <bac> rockstar: it is not part of the IProduct interface and sinzui and i have concluded it is a dangerous leftover
[21:17] <bac> rockstar: now 'getBranches' should be used
[21:17] <bac> rockstar: as a code guy please tell us if you think that's wrong for some reason
[21:18] <sinzui> rockstar: product.branches once cause some problems with snapshot, but that was a year ago
[21:18] <rockstar> bac, hm, does getBranches take any arguments?
[21:19] <bac> optional ones
[21:19] <rockstar> bac, ah, okay.
[21:19] <sinzui> visibility perhaps?
[21:19] <bac> yeah, user and status, i think
[21:19] <rockstar> Usually, I use IBranchNamespace, but that's because I never have to work with IProduct
[21:19] <bac> rockstar: but i didn't change any call sites.  no one is using that property as best i can tell
[21:20] <rockstar> I think it's a leftover that can go away.
[21:20] <bac> rockstar:  and i deployed sinzui's super-grepping prowess on the task too
[21:20] <sinzui> I think we switched from a property to a method when we introduced private branches
[21:20] <rockstar> sinzui, yeah, I would assume that privacy probably has something to do with it.
[21:21] <rockstar> Although if getBranches didn't take any arguments, I'd suggest that it should just be a property.
[21:21] <rockstar> Since it does, we'll leave it as is and just kill the property.
[21:22] <rockstar> Since IProduct.branches isn't exposed through the REST API, there doesn't seem to be any noticeable impact.
[21:22] <sinzui> rockstar: bac: annotate sees this:
[21:22] <sinzui> Tim Penhey 2009-06-11 16:20:56 +1200
[21:22] <sinzui> Remove IProduct.branches and replace with IProduct.getBranches inherited from IHasBranches.
[21:22]  * rockstar nods
[21:23] <bac> ah, so he just didn't get the other bit
[21:23] <rockstar> So it looks like thumper missed the first bit.  :)
[21:23] <thumper> ?
[21:24] <rockstar> thumper, IProduct.branches still exists, and is being removed in bac's branch.
[21:24] <rockstar> bac, are you wanting to RC this?
[21:24] <thumper> ok
[21:24] <bac> rockstar: no
[21:24] <rockstar> bac, ok.
[21:25] <rockstar> bac, r=me
[21:25] <bac> thanks!
[21:25] <sinzui> bac: rockstar: since thumper fixed the branches issue, I think we can lower the snapshot shotlist mac
[21:25] <sinzui> max
[21:26] <bac> sinzui: how low shall we go?
[21:26] <rockstar> sinzui, I defer to you on that then.
[21:26] <bac> 100?
[21:26] <bac> er, 1000?
[21:26]  * sinzui looks at annotate
[21:27] <sinzui> bacL yes, 1000 was the limit
[21:28] <bac> sinzui, rockstar: ok, i'll change it back
[21:28] <bac> to 1000
=== 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
[21:33] <sinzui> rockstar: I have a branch to review, but my QA instructions are bugus--could not complete them because a edge timeouts. I am now working on the branch to fix the edge timeouts
[22:45] <rockstar> sinzui, where is your branch?
[22:46] <sinzui> rockstar: https://code.edge.launchpad.net/~sinzui/launchpad/remove-sp-packaging-bug-272343/+merge/15617