/srv/irclogs.ubuntu.com/2009/11/19/#launchpad-reviews.txt

al-maisanintellectronica: it was worth a try :) we can meet on level 3, in the central area, I'll be there in a few minutes.00:00
intellectronicaal-maisan: oright, will join you there shortly00:00
=== 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
=== ursula is now known as Ursinha
mwhudsonthumper: hey, want to review a branch!?04:50
mwhudsonhttps://code.launchpad.net/~mwhudson/launchpad/bzr-svn-imports/+merge/15027 fwiw04:57
=== matsubara-afk is now known as matsubara
henningenoodles775: Hi! I requested a UI review of you, if that is ok! ;)12:35
noodles775henninge: np, although I was chatting with beuno and we thought that, when there's time, it'd be great to always first have a review by someone who's going through the graduation process. So if sinzui is keen to take a look first, I'll then go over it afterwards, otherwise if we don't hear back I'll do it later this afternoon.12:56
noodles775is that ok with you?12:56
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
henningehenninge: fine by me13:07
henningenoodles775: ^ :)13:12
* henninge relocates13:12
allenapnoodles775: Got time for a quick one? https://code.edge.launchpad.net/~allenap/lp-dev-utils/open-project-review-in-browser/+merge/1503713:50
noodles775allenap: sure. do we have http://api.* urls?13:56
* noodles775 has only seen http://lp.net/api/beta/*13:57
allenapnoodles775: I think the /api/beta urls are for the benefit of the js apis (can't do cross-domain stuff). The api.* domains are old-skool I think.13:57
noodles775ok.13:58
=== ursula is now known as Ursinha
noodles775allenap: done.14:10
allenapnoodles775: Thanks.14:10
gmbnoodles775: Can I get a review of https://code.edge.launchpad.net/~gmb/launchpad/no-sync-from-dupes-bug-484609/+merge/15036 please?14:23
noodles775gmb: sure.14:24
gmbTa14:24
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles || reviewing: gmb || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
gmbnoodles775: I've just spotted some errors circa lines 84 and 85 of the diff - references to multiple bug watches when there's only one. I'll fix it.14:28
noodles775gmb: ok, it's just comments in your doctest right? So I'll still start it now.14:29
gmbnoodles775: Right.14:29
noodles775gmb: s/bug_watche/bugwatch on line 80 of diff?14:37
gmbnoodles775: Yes. 14:37
noodles775gmb: also, the import of transaction on line 87 is unnecessary.14:39
gmbnoodles775: Okidoke. I'll remove it.14:40
=== sinzui changed the topic of #launchpad-reviews to: on call: noodles || reviewing: gmb || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775Hi sinzui, let me know if you can/want to do an initial ui review for henninge's branch at:14:52
noodles775https://code.launchpad.net/~henninge/launchpad/bug-422466-status-picker/+merge/1503514:52
sinzuiI do14:53
noodles775sinzui: great!14:53
sinzuiI'll do it now in fact14:53
allenapnoodles775: I've replied to your review. Thanks for the good comments. It's not urgent that you look back at it, so this isn't meant to be a "me me", just a nod :)14:56
noodles775allenap: yep, I'll try it out in a tick. Thanks.14:58
=== salgado_ is now known as salgado
gmbnoodles775: Changes made and pushed.15:05
noodles775Thanks gmb15:05
=== salgado is now known as salgado-lunch
gmbnoodles775: Thanks for the review.15:18
noodles775gmb: np!15:18
rockstarnoodles775, can I put a branch in your queue?15:21
noodles775rockstar: pop it in the queue, if I don't get to it before EODing, the next person will grab it :-)15:22
rockstarnoodles775, well, I SHOULD be the next person, but I'm at UDS.15:22
noodles775rockstar: yep, but abel will be on tomorrow too (hopefully I'll get through sinzui's and henninge's branches quickly).15:25
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles || reviewing: sinzui || queue [henninge, rockstar] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775rockstar: where is your branch? (I can't see it on activereviews...)15:29
rockstarnoodles775, intellectronica just walked up.  I can have him review so you can EOD without worry.15:29
rockstarnoodles775, I haven't submitted it.  I usually line up a reviewer before I submit.15:29
noodles775rockstar: ah ok. Great.15:30
=== EdwinGrubbs_ is now known as EdwinGrubbs
sinzuihenninge: noodles775: I did a ui review.15:38
henningesinzui: cheers15:39
noodles775Thanks sinzui, I'll do one too after your branch.15:39
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles || reviewing: henninge || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775henninge: is your branch intentionally targeted at db-devel? (or is it just that you created it when devel was closed)15:50
henningenoodles775: It depends on yui-3final15:50
noodles775henninge: sorry, maybe I missed that... is yui-3final on db-devel but not devel?15:51
henningenoodles775: yes, mars landed it into db-devel. If all works out, it will be landed on devel, too.15:52
* noodles775 is still in the process of going through email from the past week.15:52
noodles775Great, thanks henninge.15:52
henningenoodles775: I heard flacoste and mars talk about it ... ;)15:52
henningeyou were in the room, too ;-P15:52
intellectronicawhich reminds me, what's with yui3-final? we still have a few branches that depend on it waiting to land.15:53
noodles775Yeah, but I don't hear much when coding ;). But you'll land this on devel if the yui-3final lands there right? (so that it gets more exposure before release etc.)15:53
henningenoodles775: definitely15:54
henningeintellectronica: it's landed on db-devel15:54
=== noodles775 changed the topic of #launchpad-reviews to: on call: - || reviewing: henninge || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
intellectronicaand what's stopping it from landing in devel? as noodles775 said we prolly want to have it used on edge a bit before releasing it15:54
henningeask flacoste 15:55
=== matsubara is now known as matsubara-lunch
henningenoodles775: I somehow anticipated sinzui's comment about moving the edit link closer to the actual data, while pondering it over lunch. If you agree, I'll change that.15:59
noodles775henninge: I'm just getting your branch to run it locally now.16:00
henningecool16:02
noodles775henninge: but yes, looking at your screenshots (thanks for those!), I'd definitely agree that the edit icon for the item (as opposed to the status) should be next to the item itself.16:02
flacosteintellectronica: we want to do QA on staging before releasing to users on edge16:04
flacosteintellectronica: just to make sure there isn't any major fuck-up16:04
intellectronicaflacoste: oright. i'll land stuff on db-devel then. get that tested too16:05
flacosteintellectronica: plan is to land on devel on Monday16:06
intellectronicaexcellent. can't wait!16:06
bacnoodles775: are you EOD or still accepting reviews?16:12
bacsinzui: i just sent in a MP.  would you mind reviewing it?  i'll not be here for an hour, though, as i'm out to get lunch.16:16
sinzuiI do it16:17
salgadoEdwinGrubbs, you're not reviewing sidnei's combo-service branch, right?  I'm about to start reviewing it so just wanted to make sure you haven't done so16:20
=== bac is now known as bac-lunch
EdwinGrubbssalgado: I didn't get very far, so feel free to take it over.16:21
salgadook16:21
sidneisalgado: actually, mars just finished it16:23
salgadoreally?16:23
sidneisalgado: yep, right before you joined16:24
noodles775_henninge: you've got a screenshot there logged in as no-priv with the picker displaying, but when I'm logged in as no-priv I don't have an option to click on? What did I miss?16:24
henningenoodles775_: ah sorry, you need to create an entry as no-priv. Actually, create a project first, activate translations on it and upload something to the queue ...16:25
noodles775_henninge: ok, doing so now. Thanks.16:25
noodles775_henninge: also, when logged in as Carlos, are the Deleted and Blocked items in the picker meant to be disabled? They appear grayed out, but I can click on them and it updates (it could just be my colour-blindness).16:26
henningenoodles775_: I chose darkgray for those states ...16:27
henningenoodles775_: so, no, they are not grayed out.16:27
henningenoodles775_: got a suggestion for a better color scheme? ;)16:27
noodles775_henninge: nope - I'm not a great resource for choosing colours - I normally either (1) look for precedents, or (2) use a decent colour-scheme generator.16:28
henningeoh, didn't know such things exist16:30
henningenoodles775_: anyway, that is easy to fix in css.16:30
noodles775_yep.16:30
noodles775_henninge: if you don't mind flash, http://kuler.adobe.com/16:30
henningenoodles775_: wow, this is really kul!16:33
noodles775_:-)16:33
henningenoodles775_: bugs uses gray for invalid and won't fix, too.16:35
henningebut they don't have disabled items.16:35
noodles775_henninge: aha.16:35
henningesinzui: I just saw that I may have broken the jacascript behind the expander button - maybe that was why it was hard to figure out what it does.16:39
sinzuihenninge: I also tested one of my branches16:39
henningesinzui: but I agree that it looks like it should either be moved to the left of not make use of the "expander" semantics.16:40
henninges/of/or/16:40
sinzuihenninge: the expander is always on the left on gtk, qt, macos, beos, java swing, and java swt16:40
henningeas I said, I agree16:40
sinzuihenninge: I did not recognise it as an expander since it pointed to my scrollbar. I clicked the info icon and it expanded, then I had to toggle while looking at the other side of my browser to see what happened16:41
henninge;)16:42
sinzuihenninge: This is definitely a different bug16:42
leonardranyone want to review a lazr.restful branch? https://code.edge.launchpad.net/~leonardr/lazr.restful/double-your-enjoyment-2/+merge/1504516:46
flacosterockstar: i have 11 left before i disapppear in another meeting16:50
rockstarflacoste, well, can you show me some code where canonical_url hackery is done in the way that I need to do it for windmill?16:50
flacosterockstar: can you remind me of the problem again?16:50
rockstarflacoste, canonical_url returns a url without the port number on it, so it's generally useless for use in windmill tests.16:51
flacosteok16:51
rockstarflacoste, so what you said was that we need to install something into canonical_url space that tells it the url.16:52
flacosterockstar: so what you need to do is modify the WindmillLayer testSetUp and testTearDown methods16:52
flacostein testSetUp you should push the following on the config16:53
flacoste[vhost.mainsite]16:53
flacosterooturl: http://launchpad.dev:8085/16:53
flacosteand so on for all the vhosts16:53
rockstarflacoste, ah, okay.16:53
flacostelook in configs/testrunner-appserver/launchpad-lazr.conf for the whole list16:53
flacosteall vhost.* sections should be pushed16:53
flacosteand then you pop that config back in the testTearDown16:54
flacosteet voilĂ 16:54
flacostethat should work16:54
rockstarflacoste, easy peasy pumpkin squeezy.16:55
sinzuibac: r=me16:55
henningenoodles775_: pushed new version with moved edit icon and new colour scheme.16:55
noodles775_henninge: ok, merging now, thanks!16:55
noodles775_henninge: did you decide not to put the edit icon also next to the pot file (as sinzui suggested?), it only seems to be next to the template?16:58
henningenoodles775_: well, that is what is mainly been used. The file name is seldom changed.17:00
noodles775_henninge: OK, makes sense.17:00
henningenoodles775_: If we ever get as far as in-line editing all this data, it would be worth adding another icon.17:01
noodles775_henninge: well, I thought that was part of the advantage of sinzui's suggestion - that the UI wouldn't change when in-lining the other items (as you'd already have the icon there).17:02
henninge:-)17:02
henningeshot in the foot17:02
henningenoodles775_: I just tried it out and it looks too confusing. Have a look: http://people.canonical.com/~henninge/screenshots/status-picker-more-edit-icons.png17:07
henningenoodles775_: the two edit icons for different entries get dangerously close ...17:08
henningenoodles775_: I'd rather leave it with one icon per entry.17:08
noodles775_Yep, it does produce a lot of edit icons. I've written in the (as yet unsent) review that I'd leave that up to you - I think your previous justification is a good one (that it's normally only the template that users want to edit).17:09
henninge_noodles775_: daily disconnect, that is when I know it's time to go home .. ;)17:14
noodles775_henninge: yeah, I'm about to do a runner too.17:14
noodles775_just sending your review :)17:14
henninge_noodles775_: thank you very much! Enjoy your evening.17:15
noodles775_henninge, sinzui : ok, review sent, but I'm keen for sinzui's thoughts on two points mentioned in the review.17:15
noodles775_Night!17:15
=== henninge_ is now known as henninge
=== matsubara-lunch is now known as matsubara
=== bac-lunch is now known as bac
=== salgado is now known as salgado-afk
=== mwhudson_ is now known as mwhudson
=== matsubara is now known as matsubara-afk
=== abentley1 is now known as abentley

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!