| === 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 | ||
| henninge | noodles775: Guten Morgen! ;) | 08:06 |
|---|---|---|
| noodles775 | Hi hennige! | 08:06 |
| === 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 | ||
| noodles775 | :-D | 08:07 |
| henninge | noodles775: ah, you are the ocr! :) | 08:07 |
| noodles775 | Yeah, myself and al-maisan are around today. | 08:07 |
| * 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:08 |
| 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:14 |
| 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:15 |
| noodles775 | Aha. | 08:16 |
| 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:34 |
| noodles775 | Great, yep, I always do :-) | 08:35 |
| === 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 | ||
| henninge | noodles775: also, I am not sure if I am writing good js/yui code. | 08:35 |
| noodles775 | OK, I'll check it out too. | 08:36 |
| henninge | noodles775: fyi, just pushed a new version. | 08:43 |
| noodles775 | henninge: is this dependent on other changes currently in db-devel only? Or why are you targeting db-devel? | 08:45 |
| henninge | noodles775: well, I just don't want to get caught in yui3final changes by working against the latest version. | 08:46 |
| 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. | 08:47 |
| === 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 | ||
| 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:14 |
| 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:15 |
| henninge | noodles775: Ah! Yes, that was fun I had after hours yesterday ... ;) | 09:16 |
| henninge | noodles775: oh, I just read about the "unseen" css class. | 09:19 |
| henninge | windmill doc is out-of-date ... :( | 09:25 |
| 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:26 |
| 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:27 |
| 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:28 |
| henninge | noodles775: yes, sounds reasonable. | 09:29 |
| henninge | noodles775: the "unseen class does exist in style.css". Is that not generally available? | 09:29 |
| henninge | I could just move it to style-3.0.css | 09:30 |
| 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:34 |
| noodles775 | bbiab - OTP | 09:36 |
| * henninge has to go afk for 15 min | 09:44 | |
| === henninge is now known as henninge-afk | ||
| === henninge-afk is now known as henninge | ||
| 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:13 |
| 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). | 10:14 |
| === 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 | ||
| 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:15 |
| henninge | noodles775: I am reading the review atm | 10:16 |
| noodles775 | k | 10:16 |
| al-maisan | ok, thanks for letting me know. | 10:28 |
| 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. | 10:43 |
| === jelmer_ is now known as jelmer | ||
| noodles775 | gmb: sure! | 10:43 |
| gmb | Thanks | 10:43 |
| === 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 | ||
| 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:53 |
| 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:54 |
| 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:55 |
| gmb | Yeah :) | 10:57 |
| noodles775 | gmb: sent. | 11:23 |
| gmb | Cool | 11:23 |
| gmb | noodles775: Thanks; plenty for me to chew on. I'll let you know how I get on :) | 11:27 |
| noodles775 | Great, thanks. | 11:27 |
| === 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 | ||
| 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:30 |
| 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). | 12:31 |
| === mrevell-lunch is now known as mrevell | ||
| === danilo_ is now known as danilos | ||
| === henninge_ is now known as henninge | ||
| henninge | al-maisan: the branch is ready for review now. Thanks for waiting! | 14:02 |
| 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:03 |
| henninge | noodles775: sorry, didn't see your vote. Forgot to reload the mp page ;-) | 14:10 |
| === 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 | ||
| al-maisan | hello henninge, could you please take a look at http://pastebin.ubuntu.com/333878/ ? | 15:06 |
| 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:07 |
| henninge | pushing them as we "speak" ;-) | 15:08 |
| henninge | done | 15:08 |
| al-maisan | thanks | 15:08 |
| henninge | sorry about that ... | 15:08 |
| al-maisan | henninge: I also get http://pastebin.ubuntu.com/333882/ | 15:10 |
| === salgado is now known as salgado-lunch | ||
| henninge | al-maisan: haven't seen that before ... | 15:10 |
| al-maisan | hmm .. that's funny, I get it when running "bin/test --layer=TranslationsWindmillLayer --test=filter_languages" | 15:11 |
| 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:12 |
| 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:13 |
| 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:14 |
| 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:15 |
| 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:16 | |
| 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:17 |
| henninge | al-maisan: thank you very much! | 15:18 |
| al-maisan | you are welcome | 15:18 |
| === 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 | ||
| deryck | noodles775, or al-maisan -- I have a 12 line diff for review, to fix a broken js unit test. | 15:49 |
| === 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 | ||
| 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:49 |
| noodles775 | deryck: r=me | 15:50 |
| deryck | noodles775, thanks | 15:50 |
| 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 | 15:54 |
| === beuno is now known as beuno-lunch | ||
| noodles775 | EdwinGrubbs: sure! | 15:54 |
| * noodles775 is loving the 4 or 5 liners :) | 15:54 | |
| noodles775 | EdwinGrubbs: is this targeted to db-devel for a reason? (or are you planning on landing it on devel?) | 15:56 |
| 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 | 15:58 |
| * al-maisan bows out | 16:03 | |
| === 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 | ||
| bac | hi gary. could i interest you in a short lazr.lifecycle review? | 16:14 |
| bac | gary_poster: ^^ | 16:14 |
| 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 | 16:18 |
| === deryck is now known as deryck[lunch] | ||
| === salgado-lunch is now known as salgado | ||
| 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 :-) | 16:26 |
| === 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 | ||
| rockstar | noodles775, cheers! | 16:27 |
| 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:28 |
| 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:29 |
| 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:30 |
| gary_poster | bac: ok cool. ping me when you are ready | 16:31 |
| bac | gary_poster: something like: http://pastebin.ubuntu.com/333934/ | 16:45 |
| gary_poster | looking | 16:45 |
| 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:46 |
| gary_poster | bac, I approved, also with a request to update the CHANGES file (or whatever it is called) | 16:51 |
| gary_poster | oops | 16:53 |
| === matsubara-lunch is now known as matsubara | ||
| === deryck[lunch] is now known as deryck | ||
| === beuno-lunch is now known as beuno | ||
| 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:21 |
| sinzui | karma? | 18:59 |
| sinzui | beuno: rs=me | 19:00 |
| beuno | sinzui, どうもありがとう | 19:01 |
| sinzui | mr. roboto | 19:01 |
| beuno | :) | 19:02 |
| === bigjools-afk is now known as bigjools | ||
| === salgado is now known as salgado-afk | ||
| === EdwinGrubbs is now known as Edwin-lunch | ||
| bac | hey rockstar you still have another review in you? | 20:14 |
| rockstar | bac, yup | 20:23 |
| bac | rockstar: thanks - https://code.edge.launchpad.net/~bac/launchpad/bug-488762-snapshot/+merge/15628 | 20:24 |
| === matsubara is now known as matsubara-afk | ||
| bac | rockstar: in case you're interested, it uses this lazr.lifecycle change: https://code.edge.launchpad.net/~bac/lazr.lifecycle/snapshot/+merge/15612 | 20:25 |
| 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:13 |
| 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:14 |
| rockstar | bac, why did you delete the branches property in lib/lp/registry/model/product.py | 21:16 |
| 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:17 |
| 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:18 |
| 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:19 |
| 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:20 |
| 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:21 |
| 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:22 | |
| 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:23 |
| 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:24 |
| 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:25 |
| 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:26 | |
| sinzui | bacL yes, 1000 was the limit | 21:27 |
| bac | sinzui, rockstar: ok, i'll change it back | 21:28 |
| bac | to 1000 | 21:28 |
| === 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 | ||
| 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 | 21:33 |
| rockstar | sinzui, where is your branch? | 22:45 |
| sinzui | rockstar: https://code.edge.launchpad.net/~sinzui/launchpad/remove-sp-packaging-bug-272343/+merge/15617 | 22:46 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!