/srv/irclogs.ubuntu.com/2010/10/05/#launchpad-reviews.txt

wallyworldlifeless: ping04:20
lifelesshey04:27
wallyworldlifeless: i have put up for review an enhancement to allow logging of sql bind parameters, to make performance tuning easier04:28
wallyworldhttps://code.edge.launchpad.net/~wallyworld/launchpad/log-sql-bind-variables/+merge/3755204:28
wallyworldcan you take a look and see if you agree with it?04:28
wallyworldthnks :-)04:28
lifelessI reviewed it already04:28
wallyworldwow, that quick04:29
lifelessno messing :)04:29
wallyworldlet me look and see if you approved :-)04:29
wallyworldcool. thanks.04:30
bachi henninge08:12
henningeOh hi bac! You are ahead of me time-zone-wise, right ... ;-)08:12
bachenninge: thanks for the review yesterday.  i know you think i'm on crack, but the hoo-ha with the involvement portlet was following a pattern already employed.08:13
henningebac: is crack cheap in vietman?08:13
bachenninge: however, in light of your questions i'm having a hard time defending it.  :(08:13
henningebac: What would be the advantage of doing it that way?08:14
bachenninge: well, the original thought was to not repeat the code.  but i cannot rationalize why it should be in a portlet rather than the model08:15
henningeexactly ... ;-)08:15
bachenninge: you'll see in answers and maybe translations the same thing is done08:16
henningewell, it's never too late to mend our old ways. ;)08:16
henningeI mean, there is a lot of old code in LP that neither you nor I would have written that way. Non need to continue the errors of others.08:17
bachenninge: please don't joke about me + crack + vietnam.08:18
bac:)08:18
bachenninge: i completely agree08:18
henninge;)08:18
henningestub: Hi! Are you up for a database review?08:40
stubhenninge: I see a merge proposal containing nothing but sampledata rebuilt with PG 8.3 instead of PG 8.408:41
henningestub: yes, I just realized that myself.08:41
henningethe 8.3 vs. 8.4 diffference08:42
henningeHow come I am not running 8.4 locally?08:42
henningestub: also, I just added a diff for the other files that were changed.08:42
=== jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bachenninge: after installing PG8.4 you still need to change your conf files so that 8.3 doesn't start and 8.4 does08:44
henningeoh08:44
* henninge checks that 8.4 is installed ... should be08:44
bacstart.conf needs to have 'auto'08:44
jtvhenninge: even if they're both installed, you may still be running 8.3 on the expected port (5432).  Try running "psql"08:46
henningejtv: I understand and psql reports 8.308:47
jtvhenninge: that's the version of psql itself, so that tells us you have 8.3 still installed.08:47
jtvI'm trying to remember how to ask for the backend version.08:48
jtvah yes, "show server_version;"08:48
henningejtv: it is obviously 8.3 because the sampledata I generated reports 8.308:48
* henninge tries that08:48
henninge8.3.908:48
jtvhenninge: in /etc/postgresql/8.4/main/postgresql.conf, make sure that "port" is set to 5432.08:49
jtvhenninge: the package installation scripts will set it to 5433 if there's already another postgres version set to run on 5432.08:50
henningebac: start.conf in both 8.3. and 8.4 contain only one directive: "auto"08:51
henningejtv: will look at that08:51
bachenninge: yes, you must remove the one from 8.308:51
bachenninge: unless you need 8.3 for some reason you can just blow away the 8.3 config directory08:51
jtvProbably best to un-install 8.3 entirely, with --purge, yes.08:51
bachenninge: and possibly run launchpad-database-config, or whatever it is called08:52
jtvIt's a bit of a trap that doing this, by default, will leave you with 8.4 running on 5433 and all connection attempts on the default port failing as if you have no backend running.08:52
bacjtv: the script will take care of that08:52
jtvOh good08:52
StevenKlaunchpad-database-setup08:53
henningejtv, bac: I am a bit unsure what to do: http://paste.ubuntu.com/506314/09:01
bachenninge: would you mind having a look at PillarView?09:02
henningebac: sure09:02
jtvhenninge: try removing all of those that have "8.3" in them, but also installing their 8.4 equivalents in the same run.09:02
jtvhenninge: also, there's no problem with temporarily dropping the lp packages if that makes things easier.  The 8.3 stuff should go in any case.09:03
henningejtv: that's what I was more worried about09:03
henningeI think I already have all the 8.4 stuff installed.09:04
jtvhenninge: then just uninstall this stuff and then re-install the lp packages.09:04
jtvit'll probably come up with conflicts and two resolution scenarios, one using pg 8.3 and one using pg 8.4.09:05
jtv(not that I'm too familiar with aptitude)09:05
henningejtv: I don't usually use it either. I just did not find the purge option on apt-get.09:05
jtvapt-get remove --purge09:06
* henninge tries that09:06
henningejtv: much better! http://paste.ubuntu.com/506320/09:07
bachenninge: i'm hesitant to unroll all of the PillarView logic and move it to the model without discussing it with sinzui.09:08
bachenninge: i think i'd like to land what i have, repeating the existing pattern, and then fix them all together if that is what we decide to do09:08
henningebac: please do that. I did not think that it is that much.09:08
henningebac: yes, please file a bug about it.09:09
bacrt09:09
henningert?09:09
henningeright then09:09
henninge?09:09
bac"roger that"09:09
henninge:-)09:09
bacmy friend the former helicopter pilot infected everyone with saying it09:09
jtvbac: careful with that… you may trigger an allergic psychotic reaction in losas09:10
henningestub: please look at the mp again. I'll request a db review from you on it if that's ok.09:34
henningestub: oh, nm. You already did ... ;-)09:35
stubI already approved it didn't I?09:35
henningeyes, I just noticed. Thanks!09:35
lifelessI need a testfix review: https://code.edge.launchpad.net/~lifeless/launchpad/test/+merge/3757109:38
* henninge waits for diff to become available09:39
henningelifeless: simple enough fix, thank you, but can you please rename _setup to _is_setup or _is_set_up?09:43
henningelifeless: r=me09:43
=== noodles775 changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jtvnoodles775: I can trade you a review in a few minutes10:02
jtv(well, it'll take more than "a few minutes" before you reap the benefits, but still :)10:02
noodles775jtv: great!10:03
henningejtv: can you have a quick look at this code-wise first, please?10:05
henningehttps://code.edge.launchpad.net/~henninge/launchpad/db-devel-translationmessage-pofile/+merge/3756310:05
jtvI think I can promise a quick look…10:05
jtv2749 lines of diff… which of those need my quick look?  :)10:06
henningejtv: 2456-247810:07
henninge:-P10:07
jtvhenninge: grrr truncated diff10:08
henningejtv: look in the comments!10:09
henningethe rest is just removing lots of 'pofile' and 'NULL' from sampledata10:09
jtvhenninge: I have to _get_ at the comments first…10:10
henninge? They are at the top, above the diff, are they not?10:10
henningeoh, you are going through the mails ...10:11
jtvhenninge: oh, the MP comments.  I thought you meant code comments.10:13
henningejtv: there is this https://launchpadlibrarian.net/57120537/review.diff and this https://launchpadlibrarian.net/57122440/inc.diff10:14
henningejtv: but you can see it all on the mp page.10:14
jtvhenninge: yes, I see10:14
jtvhenninge: wow, passing vars() to a % is pretty nasty… didn't know we did that.  For a doctest I guess it doesn't matter that much.10:19
henningejtv: Not using the factory here seems pretty out of place, too.10:20
henningeBut fixing that is out of scope for this db-oriented branch.10:20
jtvhenninge: ultimately most or all of our doctests should just go away and leave us in peace.10:20
jtvanyway, approved.10:20
henningejtv: thanks10:20
jtvnoodles775: getting to your branch now10:20
noodles775jtv: great, thanks. I've just reviewed yours.10:21
jtvthanks10:21
jtvnoodles775: the "dynamic name for the action" you refer to is the text in the button at the bottom, right?10:24
noodles775jtv: yeah, so that it displays "Sync selected Warty versions into Hoary" (with appropriate substitutions).10:24
jtvgotcha10:24
jtvI never thought about that; didn't think it'd have to come out of a vocabulary.10:25
noodles775huh? the dynamic vocabulary issue is a separate one... what did you mean exactly?10:25
jtvnoodles775: ah, that makes more sense10:25
jtvJust trying to parse your MP with a bit of a headache.  :)10:26
noodles775urgh... hope it's not too painful.10:26
bacthanks for the rewording suggestions, mrevell10:26
jtvnoodles775: no, I'll just take it out on some poor engineer in a review.10:26
noodles775:P10:26
jtvnoodles775: it goes like this… I thought point 2) in the MP detailed your approach to solving point 1), though there's nothing in particular to indicate that.10:27
bacmrevell: should we really say "Sorry" though?10:27
=== bigjools changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv, noodles775,bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jtvnoodles775: I'm not much at home in zope but istm what you want is an alternative to custom_widget that you can invoke from, say, __init__10:31
noodles775jtv: well, the vocabularies are actually specified on the field, not the widget (the widget then grabs them from the field).10:32
noodles775jtv: IMO, if zope had something like the IContextSourceBinder, IViewSourceBinder, that would work.10:33
noodles775AFAICS, zope does allow a vocab factory to implement ISource instead, which I had thought in the past was a more general way that would allow what I needed, but I've never managed to get it to initialise from the view.10:34
noodles775Hence, the solution I used in this branch.10:34
jtvnoodles775: this sounds familiar10:34
noodles775I might ask one of the zope guys to take a look later when they are around, incase there is something I've missed.10:35
jtvnoodles775: smart move10:35
bacmrevell: ping10:36
mrevellhey there bac10:37
bacmrevell: i have a counter proposal10:37
bachowzabout:10:37
bac Sorry, you can't report a bug for the $ProjectGroup project group. None of the10:37
bac projects within the group use Launchpad for bug tracking. Please check the10:37
bac $ProjectGroup website for details of where they track bug reports.10:37
bacargh, no, that's yours10:37
jtvnoodles775: it's really galling that you have to go into class attributes to get at the label…  I don't suppose you could somehow _easily_ set self.actions to a copy of self.__class__.actions and manipulate that instead?10:37
mrevellhaha, I thought it looked good bac10:37
bacYou cannot report a bug for <tal:name replace="name"/> as none of10:37
bac            the projects within the group use Launchpad for bug tracking.10:37
bac            Please check the individual projects for details10:37
bac            of where bugs are reported.10:37
baca) we don't know if the project group has a web site10:38
bacb) i didn't like starting off with "Sorry"10:38
bacit is a shame but it isn't my fault so why should i apologize?10:38
* mrevell is British10:39
mrevellthat's all the explanation you need10:39
mrevellbac: +1 on your version.10:39
mrevellI like it10:39
bacyes, and when i read that i heard it in your voice10:39
noodles775jtv: agreed, but no, I didn't see an obvious easier way, hence my question on the MP. I'll take another look though.10:39
jtvas a Dutch poet ones wrote about theBritish: "mylord's final word is: please excuse my rigor mortis"10:39
mrevell:)10:39
jtvs/ones/once/10:39
bacthanks mrevell10:40
mrevellYou're welcome.10:40
mrevellOr, as Americans seem to prefer, "Mmm hmm".10:40
jtvnoodles775: I wonder if it's possible to use a factory instead of a class for the view.10:41
bacmrevell: also, i'm concerned about using the project display name in that sentence10:42
bacmrevell: sometimes it will be "Mozilla" and others "Bazaar Project"10:42
mrevellbac: Oh, because people put hard-to-parse things as their display name.10:42
jtv(little Robbie Tables as we call him)10:43
mrevellheh10:43
bacmrevell: not hard to parse but sometimes it would need a 'the' and others not10:43
bacwft do we call "the"?10:43
mrevellarticle?10:43
bacah, yes, an article10:43
jtvlovely language, English: it has "the" definite article but your choice of "an" indefinite article.10:44
bacjtv:shush, we're working here10:44
jtvnoodles775: completely different thing meanwhile… ll. 74—79 doesn't format the list comprehension quite right.10:44
* jtv shushes10:44
noodles775jtv: line 79 should be indented?10:45
bacmrevell: in the project registration we do encourage them to end with "Project", so we should word for that?10:45
mrevellbac: It's a little awkward but how about this? "You cannot report a bug for the project group '<tal:name replace="name"/>'"?10:45
jtvnoodles775: I think this one will become easier if you keep the list comprehension in a variable.10:46
jtvnoodles775: we're supposed to break the line after the opening bracket, not before.10:46
noodles775Right, I missed that.10:47
bacmrevell: yeah, that is awkward10:47
mrevellbac: I suppose we don't have to explicitly state the project group name. *Presumably* the person knows which project group they're trying to report a bug on ... but I'm always a fan of being explicit because I know it's easy to confuse people when you're not.10:48
jtvnoodles775: in line 159 of the diff, s/syncs/sync/ ?10:50
noodles775jtv: yep.10:50
jtv(given how little I know about this, either way could have been valid)10:51
jtvnoodles775: I didn't know you could construct a BeautifulSoup out of a view.  That's neat.10:52
jtvbrb10:53
=== jtv is now known as jtv-afk
noodles775Yeah, I forget where I saw it first, but it's handy for unit-tesing special case things in the template.10:53
henningebac: please push the latest revision of your branch. ;)10:53
bachenninge: ok, i was just about to10:54
bachenninge: why so pushy?  :)10:54
henningeGTD ;-P10:54
bacmrevell: i looked at how it was used in context other places in that template and just stuck with what i wrote originally10:55
mrevellOkay.10:55
bachenninge, mrevell: could  you update the MP, please?11:05
bachenninge: what about the UI?  did you consider it as a UI reviewer or should i just run it past sinzui?11:06
henningebac: no, I have not but let me have a look.11:07
bachenninge: for the MP you'll have to state "code/UI*" or something like that11:08
bacwhich will no doubt confuse our tools11:08
henningebac: oh, I cannot do two different reviews?11:08
henningenever tried it before11:08
bachenninge: nope, it takes the latest type11:08
henninge:(11:09
henningebac: I'll just comment on the ui, then.11:09
bachenninge: i think a combined type would be better11:09
bacfwiw11:09
henningeeven if it confuses our tools? ;)11:10
henningebac: what's the url for seeing and adding projectgroups?11:12
henningeI forgot11:12
=== jtv-afk is now known as jtv
bachenninge: we hid it at lp.net/projectgroups11:12
henningeah, I tried +projectgroups11:13
henningeI sense a bit of inconsistency here ... ;)11:13
jtvthere's also /builders.  I keep typing /+builders instead.11:13
jtvI wonder if I could call my project "builders"11:14
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [jtv, noodles775,bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bachenninge: lp.net/people, lp.net/projects, lp.net/distros ...11:21
henningebac: then maybe t.lp.net/+translationgroups is the inconsistency?11:22
henningeand +languages?11:22
henningeI know, they are not pillars but do the users see the difference?11:22
henningebac: have a look at this, please:11:29
henningehttps://bugs.staging.launchpad.net/mailclipper11:29
henningenm, I'll post a screenshot11:33
bachenninge: what is your question about mailclipper?11:37
henningebac: that's how uncofigured bugtracker looks on a project11:38
henningehttp://people.canonical.com/~henninge/screenshots/unconfigured_bugtracker.png11:39
henningeand here is how an unconfigured blueprints looks on a project group:11:39
henningehttp://people.canonical.com/~henninge/screenshots/unconfigured_blueprints_pg.png11:39
henningebac: so your screenshots is at least missing a page heading and the breadcrumbs.11:40
henningebut I also like how noodles (I think it was him) added those help links on the project page.11:40
jtvnoodles775: I approved your code.  Tried some more to find alternatives for the dynamic label, but no luck.  I do hope you find something.11:41
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: jtv || queue: [noodles775,bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jtvhi gmb—I think noodles775 just reviewed me.11:41
jtvgmb: …and I just reviewed noodles775.11:42
gmbHeh.11:42
gmbnm; I'll start on bigjools instead11:42
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: bigjools || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bigjoolsit's trivial11:43
gmbbigjools: Indeed. It's approved.11:44
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jtvgmb: fast work :)11:46
gmbSmall branch :)11:51
gmbAnd lots of people doing work for me.11:51
gmbAlways fun to be fashionably late...11:51
noodles775Thanks jtv11:58
jtvnp, thank you too11:58
noodles775gary_poster: Hi! I'm wondering if you'd be able to look at two zope questions at https://code.edge.launchpad.net/~michael.nelson/launchpad/652838-select-diffs-for-syncing/+merge/3757212:18
gary_posternoodles775: on call, will look, formlib may in fact suck at this12:20
noodles775Thanks gary_poster, no rush :)12:20
bachi gmb, i'm working on the bugs index page for project groups that do not use launchpad.  henninge has pointed out that page doesn't12:21
bachave a title or breadcrumbs12:21
baci see that the project group page never had either.  do you know why?12:21
bacderyck: perhaps you know about ^^^12:22
gmbbac: No, I don't;12:22
gmbMy first guess would be "we didn't have time before 3.0 and then other things got in the way"12:23
bacgmb: probably.12:23
bacgmb: so you wouldn't be hurt if i added such things?12:23
bacif i can figure it out, that is...12:24
deryckbac, yeah, no good reason.  Just oversight.12:24
gmbbac: I don't think it would hurt the page. It may hurt your brain; I can't remember how knotty the +index pages are...12:25
bacgmb:  very convoluted12:25
henningebac: you should have an example in the blueprint page for project groups, or not?12:26
bachenninge: you'd think.  this bug page is complicated12:26
henninge:(12:26
gmbYeah, don't make the mistake of thinking that just because we're not Soyuz we can't write curiously intricate* code.12:31
gmb*replace with "spaghetti" as appropriate12:31
leonardrgmb, can you do ui reviews?12:34
gmbleonardr: 'fraid not.12:37
leonardrgmb, np. is it okay to get the non-ui code reviewed before the ui is approved? (i have actually never done a ui review before)12:37
gmbleonardr: Sure, that's fine.12:39
noodles775leonardr: also, you can request a review from launchpad-ui-reviewers and then ping someone (henninge and salgado are currently mentats, so may want some more).12:39
leonardrnoodles775: edwin already reviewd it, so i need a non-mentat reviewer to check his review12:40
noodles775leonardr: ah, who's his mentor, or does he not have one?12:41
leonardrnoodles775: i got edwin's review about 2 weeks ago and then lifeless put the brakes on the branch, so i don't really remember anything. i may just start over12:42
leonardrgmb, the branch is https://code.edge.launchpad.net/~leonardr/launchpad/rename-grant-permissions/+merge/3636312:46
leonardredwin, when you get in, let me know who your mentor is12:46
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
leonardrhmm, that diff looks like there's a branch i haven't merged in or something12:48
leonardrgmb, hold off a bit12:49
* gmb holds12:50
leonardrok, the problem was i'd proposed a merge against db-devel instead of devel. but now i see a two-week-old XXX in the diff, so i may be a while longer12:55
leonardrgmb, if you're almost at eod, don't wait around for me12:55
gmbleonardr: It's just coming up to 1pm for me, so I'll go and grab lunch. If you're still going on this at about 5pm my time I'll let you know that I'm EODing :)12:56
leonardrok12:56
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbleonardr: I've taken your branch out of the queue for now; just re-add it when it's ready.13:02
leonardrgmb. thanks13:02
=== matsubara-afk is now known as matsubara
noodles775henninge or salgado: do either of you have time for a UI review? https://code.edge.launchpad.net/~michael.nelson/launchpad/652838-select-diffs-for-syncing/+merge/3757213:07
noodles775I don't think there's much to review here, it's just standard selection/form submission.13:07
salgadonoodles775, I'll do it in a couple hours if henninge doesn't beat me to it13:30
noodles775Thanks salgado.13:31
* henninge is not the beating type ...13:31
noodles775l13:41
noodles775heh, another great use of the posture sensing ubuntu interface would be to optionally ensure the window you're looking at has focus (if you haven't explicitely given focus to a window in the last n seconds).13:42
=== leonardr changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: lunch || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
leonardrhenninge, can you do a ui review for me?14:18
leonardrhttps://code.edge.launchpad.net/~leonardr/launchpad/rename-grant-permissions/+merge/3759014:18
leonardredwin did a review about two weeks ago, as recorded in the mp linked to from that me14:18
leonardror salgado, you can do my ui review after noodles755's14:20
leonardrhttps://code.edge.launchpad.net/~leonardr/launchpad/rename-grant-permissions/+merge/3759014:20
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbleonardr: Is your branch ready for code review now, too?14:21
leonardrgmb: yes it is14:22
gmbleonardr: Okay, I'll look shortly.14:23
leonardrgreat14:23
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
salgadohenninge, can you take leonard's?14:31
leonardrgmb, pushing a tiny test refactoring (r11591)14:37
gmbk14:38
henningesalgado, leonardr: yup, in 15 min.14:39
gmbleonardr: code is r=me.14:54
gmbleonardr: Nice work, BTW :)14:54
leonardrgmb, thanks14:54
henningeleonardr: looking now14:57
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
sinzuileonardr, ping15:17
leonardrsinzui, hi15:19
sinzuiI am reading your doctest15:20
leonardrok15:20
sinzuileonardr, "The Ubuntu desktop called mycomputer" <- how do you know I am not running Mint or Fedora15:21
leonardrsinzui: it's the client's responsibility to put the right string in there15:23
leonardrthat said, i have no idea how the client is supposed to know15:23
sinzuiokay15:24
leonardri'm kind of thinking uname -o but that gives "GNU/Linux" (which might be good enough?)15:24
sinzuileonardr, "all applications running on mycomputer" <- I think this is an exaggeration. All applications running when I am logged in is closer to the truth15:25
sinzuiThis is hard because most users do not distinguish between sessions and that procs run as under multiple users15:26
leonardrsinzui: yes, i would say that knowing the truth is a prerequisite to knowing that that's an exaggeration15:26
sinzui:)15:26
sinzuiuse "choose" instead of "click". I can use a touch screen or keyboard15:27
sinzuileonardr, if we used the exaggerated language, how will we respond to the bug reports from the knowledgeable?15:29
leonardrsinzui: we tell them the whole truth and say that the wording is geared towards the casual user15:31
leonardrsinzui: platform.linux_distribution() will give me 'Ubuntu'15:31
leonardrbut it is a 2.6ism15:32
sinzuifab15:32
leonardrso i guess i will use platform.dist() instead15:32
sinzuileonardr, Other that the removal of the word "click" I have no suggested changes.15:34
leonardrsinzui: ok. is this an official ui review?15:35
sinzuiyes15:35
sinzuiI will approve it now15:35
henningeleonardr: I was about to tell you that I cannot reach the page ...15:37
henningeleonardr: I get an openid login page and then "you're not allowed here".15:38
henningeI don't see a warning15:39
leonardrhenninge: this is not a page you can just visit in your web browser15:39
leonardra client such as launchpadlib needs to do some setup work first15:40
leonardrit's not a normal part of launchpad15:40
henningeleonardr: yes, that is what I get when I do that.15:40
=== mars changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
henningebrowser pops up with a login page.15:40
noodles775salgado: thanks. And yes, the all/none checkbox is included on the mockup, I hope to add it in a later branch, but it's not a priority right now.15:40
marsmorning gmb15:40
noodles775salgado: who's your ui mentor again?15:40
leonardrhenninge: walk me through it step by step. what do you do?15:41
salgadonoodles775, sinzui15:41
gmbHi mars15:41
henningeleonardr: 1. "make run" in your branch15:42
noodles775sinzui: do you have time? https://code.edge.launchpad.net/~michael.nelson/launchpad/652838-select-diffs-for-syncing/+merge/3757215:42
henningeleonardr: 2. "python" in another terminal15:42
henningeleonardr: that's a system python15:42
sinzuinoodles775, I am just finishing leonardr's review. I will start yours in a few minutes15:42
noodles775Thanks sinzui.15:42
henningeleonardr: 3. "from launchpadlib.launchpad import Launchpad"15:43
=== allenap changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: - || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
henningeleonardr: 4. "l = Launchpad.login_with("Ubuntu desktop (Bob's Computer)", service_root="dev", allow_access_levels=["DESKTOP_INTEGRATION"])"15:43
allenapgmb, mars: Either of you fancy a short branch?15:43
gmbmars: Could you take allenap's branch?15:43
marssure15:43
gmbI'm trying not to context switch.15:43
gmbmars: Thanks.15:43
marsallenap, what do you have?15:43
leonardrhenninge: you want "System-wide: Ubuntu desktop (Bob's Computer)"15:44
allenapmars: https://code.edge.launchpad.net/~allenap/launchpad/structural-subscriptions-with-filters-5/+merge/3760815:44
=== mars changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -,allenap || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
marsallenap, I assume this is the branch listed in the topic?15:45
henningeleonardr: ah, I took that from the original MP but I admit I did not read through the comments.15:45
leonardrhenninge: yeah, it changed so that you could say "System-wide: Android phone (...)" instead of always having to say "desktop"15:45
allenapmars: Yep.15:45
allenapThank you :)15:45
henningeleonardr: ok, I see it now ;)15:45
leonardrgreat15:47
leonardrhenninge, fyi, sinzui just did the ui review15:48
marsallenap, done, r=mars.15:48
=== mars changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
allenapmars: Cheers!15:48
henningeleonardr: I saw that but I have two more comments. I just put that in there.15:49
leonardrok15:49
leonardrthanks15:49
bdmurraymars: could you review https://code.edge.launchpad.net/~brian-murray/launchpad/bug-supervisor-permissions/+merge/37514 ?15:49
marsbdmurray, sure15:50
=== mars changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -,bdmurray || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
henningeleonardr: btw, a screenshot would have sped this up a lot. ;-)15:50
leonardrhenninge: sorry, i almost never do anything that has any ui component, so i'm new to this15:51
henningeleonardr: np15:51
sinzuinoodles775, you have my approve with a remark about button case15:52
leonardrmrevell, can i ask you to look at the wording in the branch https://code.edge.launchpad.net/~leonardr/launchpad/rename-grant-permissions/+merge/37590 ?15:55
mrevellI'd be very happy to leonardr.15:56
noodles775Thanks sinzui15:57
marsbdmurray, done, r=mars16:01
=== mars changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bdmurraymars: great, thanks!16:02
henningemrevell, leonardr: screenshot for your convenience: http://people.canonical.com/~henninge/screenshots/desktop-integration-warning.png16:08
henninge:-)16:08
leonardrmrevell: i'd also like your opinion on a good heading for the page16:08
mrevellthanks henninge, have you added me to the review?16:08
mrevellI'd be glad to.16:08
henningemrevell: done16:09
=== gmb changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== deryck is now known as deryck[lunch]
leonardrhenninge, is there any special technique for showing a page's heading, or shall i just scrape the h1 tag?16:43
henningeleonardr: no, I think it's just an h1.16:46
henningethe style sheet will take care of the rest16:46
henningeleonardr: but put it inside the top-portlet container.16:47
leonardrhenninge: i meant for testing that it has a certain value16:47
henningeleonardr: I don't understand16:47
leonardrhenninge: if i put <h1>Integrating $computer into your Launchpad account</h1> in the template, i need to add a test that "Integrating Bob's Computer into your launchpad account" shows up16:48
henningeleonardr: I'd just get the top-portlet and do "print extract_text"16:50
henningeextract_text(tag)16:50
leonardrok16:50
=== deryck[lunch] is now known as deryck
=== matsubara is now known as matsubara-lunch
leonardrhenninge: r11592 of my branch has the change you asked for17:12
leonardrmrevell: the title of the page i just added is "Integrating [computer/application] into your Launchpad account"17:12
gary_postermars, hopefully easy one when you get a chance: https://code.edge.launchpad.net/~gary/launchpad/bug650343/+merge/3763317:23
marsgary_poster, sure17:25
gary_posterthanks17:25
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: gary_poster || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
henningeleonardr: thank you very much. This looks good.17:26
henningenow we are just waiting for mrevell to give his blessing ;-)17:26
marsgary_poster, did you link the correct bug?17:26
mrevellhenninge: I'm clear now :)17:27
marsgary_poster, bug 6504317:27
_mup_Bug #65043: latest updates to intel-agp and/or i915-drm  in 2.6.17-10.29 slows down x generates mammoth syslog files <linux-source-2.6.17> <linux (Ubuntu):Incomplete> <linux-source-2.6.17 (Ubuntu):Won't Fix> <https://launchpad.net/bugs/65043>17:27
gary_postermars, sorry, yes and no.  See the programmatic bug link underneath my text17:27
gary_posterbug 65034317:27
_mup_Bug #650343: Add X-Launchpad-Original-To to recipient lists <Launchpad Foundations:Triaged by gary> <https://launchpad.net/bugs/650343>17:27
marsah, ok17:28
gary_postercorrected17:28
=== salgado is now known as salgado-lunch
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
marsgary_poster, approved, r=mars17:40
gary_postermars, thank you.17:40
gary_posteryou ask good questions.  my answer to #1: I have no idea, but I think that the log warnings are intentional.  this was largely a mechanical change for me.  How far do you think I should chase this?  TBH, if you do not push me, I will not chase it at all.  I'm not proud of that, but I want to get it off my queue. :-)  So feel free to push me.17:40
gary_poster#2: No idea, but that sounded like long-term thinking AFAICT17:41
marsgary_poster, just a #comment will do - something to say "Yes, I thought about these, here is what I think you should know about them"17:41
gary_posterOK, I'll see if I can crib an answer from someone, mars.  thanks17:42
marsgary_poster, #2 is long-term thinking, yes.  Part of our review initiative to move tests out of c.l.17:42
gary_postergotcha17:42
=== matsubara-lunch is now known as matsubara
=== gary_poster is now known as gary-lunch
=== salgado-lunch is now known as salgado
gary-lunchmars, jml, I'm not clear on the requirements: I need to move c/l/mail/incoming.py and c/l/doc/incomingmail.txt to lp/services/mail in order to land?  Or just the .txt?19:17
gary-lunchbac, ping?19:25
=== gary-lunch is now known as gary_poster
* gary_poster forgot to change his nick19:25
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gary_postermars, did you see my question above?  I tried to find instructions on what needs to move where but didn't find anything.20:12
gary_posterand it looks like jml is telling me to move at least one file20:12
marsgary_poster, yes, I don't know - I think the .py files would be easiest to move20:13
gary_posterwell, except then I have to fix all imports20:13
gary_posterit just makes this a much bigger branch20:13
marsyes, unfortunately.  If you had a pipes setup, I would say you could do that, or a follow-up20:14
marschanging the imports should not be too difficult20:14
marsgrep + ec2 will find everything20:14
gary_posteryeah, a follow-up branch is what I was thinking20:14
gary_posteryeah, slowly ;-)20:15
gary_posterokthanks20:15
marsnp20:15
allenapmars: Have you got time for another small branch? https://code.edge.launchpad.net/~allenap/launchpad/structural-subscriptions-with-filters-6/+merge/3765320:28
marsI do20:28
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: allnap || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: allenap || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
allenapmars: Tip top, thanks :)20:30
marsallenap, done, r=mars20:33
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
allenapmars: Thank you!20:33
=== matsubara is now known as matsubara-afk
=== mars changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews

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