/srv/irclogs.ubuntu.com/2010/01/20/#launchpad-reviews.txt

=== jamalta-afk is now known as jamalta
rockstaranyone around for a few reviews?02:32
jamaltathanks bac 03:06
bacjamalta: np.  sorry i didn't get to it earlier.  (had to go to the movie!)03:06
jamaltabac: heh no worries, i'm sure there is no rush with a trivial bug like that :)03:06
=== bac 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
jamaltaconsidering the majority of users won't even see that dialog ;)03:07
bacnope.  good work still.03:07
jamaltathanks03:09
=== jamalta is now known as jamalta-afk
=== Ursinha is now known as Ursinha-afk
mrevellAnyone wanna take a look at a small tour text change branch? https://code.edge.launchpad.net/~matthew.revell/launchpad/tour-commercial-tweak-bug-393348/+merge/1766609:46
=== matsubara-afk is now known as matsubara
jamalta-afkwell, looks like some people didn't like my change :( (bug #509791)14:04
mupBug #509791: "sources.changes" is not a well-known term <Soyuz:Triaged by julian-edwards> <https://launchpad.net/bugs/509791>14:04
=== jamalta-afk is now known as jamalta
jamaltai guess i'll just make the change to "changes file" ...14:08
leonardrgary, want to start the day off with a quick review?14:13
leonardrhttps://code.edge.launchpad.net/~leonardr/lazr.restful/multiversion-rename-params/+merge/1774114:13
gary_posterleonardr sure14:14
jamaltacould someone approve the change to "changes file" instead of "sources.changes" from bug 509791 so i can push this branch really quick?14:24
mupBug #509791: "sources.changes" is not a well-known term <Soyuz:Triaged by julian-edwards> <https://launchpad.net/bugs/509791>14:24
=== henninge_ is now known as henninge
=== mrevell-lunch is now known as mrevell
jamaltabigjools: https://code.edge.launchpad.net/~jamalta/launchpad/changesfile-50979114:34
=== jamalta 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
=== jamalta changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [jamalta] || This channel islogged:http://irclogs.ubuntu.com||https://code.edge.launchpad.net/launchpad/+activereviews
=== Ursinha-afk is now known as Ursinha
=== salgado is now known as salgado-afk
=== salgado-afk is now known as salgado-lunch
=== sinzui changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [jamalta, sinzui] || This channel islogged:http://irclogs.ubuntu.com||https://code.edge.launchpad.net/launchpad/+activereviews
bigjoolsjamalta: looking14:50
jamaltabigjools: sorry about that, i tried to make sure i used a good term to replace changesfile with :(14:51
bigjoolsjamalta: np, easily fixed!14:52
bigjoolsapproved, thank you, I'll land it shortly14:52
jamaltabigjools: yeah, specially since i already had a diff with the changes to sources.changes14:52
jamaltabigjools: thanks! glad to hear this is better :)14:52
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [jamalta, sinzui] || This channel islogged:http://irclogs.ubuntu.com||https://code.edge.launchpad.net/launchpad/+activereviews
gary_posterleonardr: I'd like to see an example of the typical use of getting a given version of the declarations.  All I see in webservice-declarations is using pop, which doesn't seem likely to be the usual usage.  Is the expectation that you will say special_dict.stack[version_name][name that you want]?15:09
leonardrgary: no, i'm actually going to be using pop15:10
leonardri'm going to create the most recent version, then the version before that, etc15:10
gary_posterleonardr: oh, huh.  So this is really a very transient data structure15:11
gary_posterok15:11
gary_posterleonardr: the only other comment I've scribbled into the text box so far is that, for the name, VersionedDict makes sense to me, particularly with new approach.  OTOH, BleedThroughDict has the vampire-ish connotations that are so in vogue.15:11
=== jamalta changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [sinzui] || This channelislogged:http://irclogs.ubuntu.com||https://code.edge.launchpad.net/launchpad/+activereviews
leonardrVersionedDict is good15:11
gary_posterleonardr: ok cool.  I'll approve, with that suggestion.15:12
rockstarEdwinGrubbs, do you mind if I chuck some branches in for review (I have 4 of them).15:21
EdwinGrubbsrockstar: go for it15:22
=== matsubara is now known as matsubara-lunch
rockstarEdwinGrubbs, https://code.edge.launchpad.net/~rockstar/launchpad/upgrade-job-2.0-changes/+merge/1770115:28
rockstarhttps://code.edge.launchpad.net/~rockstar/launchpad/upgrade-ui/+merge/1770415:28
rockstarhttps://code.edge.launchpad.net/~rockstar/launchpad/pedantry-round2/+merge/1770515:28
rockstarhttps://code.edge.launchpad.net/~rockstar/launchpad/pedantry-round3/+merge/1770615:29
gary_posterleonardr: approved, with some small comments15:29
leonardrgary: i may get rid of the class in the future, but who knows--it might get more complicated before this is all over15:36
gary_posterleonardr: ack, cool15:38
=== Ursinha changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [sinzui, Ursinha] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
henningemrevell: can you please revert lib/lp/translations/browser/product.py in your branch? Ursula is fixing that in hers.15:39
mrevellhenninge, My branch won't pass the test suite if I revert it, though, won't it?15:40
henningemrevell: no, they are just warning atm.15:41
=== leonardr is now known as leonardr-market
mrevellah, okay, thanks henninge15:41
=== salgado-lunch is now known as salgado
henningebac: I am not sure now, actually. As sinzui's branch has landed, are import violations still warnings?15:43
sinzuihenninge: There were some that were introduced after I landed my branch15:43
henningesinzui: the question is if the test suite will pass if the branch has import violations.15:44
sinzuiyes, that is how we got so many in the first place15:44
sinzuihenninge: there is one import violation in devel:15:45
sinzuiThere were 1 imports of names not appearing in the __all__.15:45
sinzuiYou should not import IProductSeries from lp.registry.interfaces.product:15:45
sinzui    lp.translations.browser.product15:45
henningesinzui: that's the one we are talking about ... ;-)15:45
EdwinGrubbssinzui: do you still need someone to review your needs-linking-bug-507937 branch?15:45
Ursinhasinzui: I just fixed that, as mrevell 15:45
Ursinha:)15:45
sinzuiEdwinGrubbs: yes please15:46
henningemrevell: didn't I review this branch before?15:46
mrevellhenninge, no, this is a new branch15:46
henningemrevell: but it also renames a button from Continue to Close. That looks awfully familiar.15:47
henningeDid you not land the first branch?15:47
mrevellhenninge, Ah, I see. I merged that first branch into this one as I've had trouble getting it through the test suite, but needed the changes there. I'll land that branch and then ask you to review this new one.15:48
henningemrevell: you can add that branch as a pre-requisite when creating the mp for the new one and it will create a correct diff.15:49
mrevellAh, just a moment then15:49
=== bigjools is now known as bigjools-otp
=== rockstar changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [sinzui, Ursinha, rockstar, rockstar, rockstar, rockstar] || This channelislogged:http://irclogs.ubuntu.com||https://code.edge.launchpad.net/launchpad/+activereviews
bachenninge: yeah, they are still warnings16:10
bachenninge: but that particular one will be fixed in many branches.16:11
henningebac: we weren't sure earlier but is that likely to cause merge conflicts16:16
henninge16:16
henninge?16:16
henninge^16:16
bachenninge: probably16:17
henningeso that's why I asked Matt to back it out.16:20
henningebac: do you know of a branch that has the fix that is about to land?16:20
bachenninge: i don't.  it's complicated to try to shepherd through.  perhaps it would be best just to do a simple rs branch to fix it16:21
henningebac, I guess that's true. rs=bac?16:22
bachenninge: sure16:22
bacit's a two line fix...16:22
henningeI know ...16:24
=== bigjools-otp is now known as bigjools
henningemrevell: so you only show the popup on the dashboard? What about the +translate page?16:28
mrevellhenninge, I'm not sure there's a suitable place for it on the +translate page. What do you think?16:29
henningemrevell: have you seen the balloon?16:30
henningemrevell: I wonder if we could just put another (not dismissiable) one on top of that "Translating for the first time? Read the tips for new translators."16:31
henningefor no-karma people.16:31
mrevellhenninge, that'd be ideal. I still don't see the balloon -- where is it on this page, for example? https://translations.edge.launchpad.net/ubuntu/karmic/+source/gfxboot-theme-ubuntu/+pots/bootloader/en_GB/+translate16:31
=== matsubara-lunch is now known as matsubara
henningemrevell: the en_GB team doesn't have any instructions, either.16:35
henningemrevell: see the German version:16:36
henningehttps://translations.edge.launchpad.net/ubuntu/karmic/+source/gfxboot-theme-ubuntu/+pots/bootloader/de/+translate16:36
mrevellheh, ok16:36
* mrevell looks16:36
mrevellAh, yes, that'd be perfect henninge16:37
henningemrevell: only I'd not make it dismissible because that requires Javascript.16:38
henningemrevell: It is dismissed when people start translating.16:38
mrevellSounds good16:38
henningecool16:39
henningemrevell: BUT16:39
henningewe do need an UI review for this ...16:39
mrevellhenninge, right16:39
henningetwo stacked balloons, maybe martin does not like that.16:39
henningemrevell: I think both ui reviewers (I know of) are unavailable today.16:40
henningebut there are a few ui* reviewers on the schedule.16:40
=== Ursinha is now known as Ursinha-nom
intellectronicaEdwinGrubbs: busy review shift, eh?17:10
EdwinGrubbsintellectronica: it seems like it17:11
intellectronicaEdwinGrubbs: i wanted to ask for another review, but i'll try to see if someone else can do it17:11
intellectronicaderyck: https://code.edge.launchpad.net/~intellectronica/launchpad/patch-in-mailnotification/+merge/1776117:16
EdwinGrubbssinzui: is the hotness being updated by triggers yet?17:17
sinzuiI do not know. I do not think that is a issue in my branch17:18
=== leonardr-market is now known as leonardr
bacsinzui: i've been reviewing your comments on https://code.edge.launchpad.net/~bac/launchpad/bug-499351-batching-dls/+merge/17492 together with beuno's.  i think i need to have a call after lunch to figure it out.17:39
=== adiroiban changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [sinzui, Ursinha, rockstar, rockstar, rockstar, rockstar,adiroiban(bug-359180-take-2)] || This channelislogged:http://irclogs.ubuntu.com||https://code.edge.launchpad.net/launchpad/+activereviews
=== adiroiban changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: - || queue [sinzui, Ursinha, rockstar, rockstar, rockstar, rockstar,adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuibac: okay17:47
rockstarEdwinGrubbs, does the topic reflect your review progress?17:51
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: sinzui || queue [Ursinha, rockstar, rockstar, rockstar, rockstar,adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
EdwinGrubbsrockstar: now it does17:51
rockstarEdwinGrubbs, okay, thanks.17:52
EdwinGrubbsUrsinha-nom: do you need someone to review lp:~ursinha/launchpad/bug422056-add-translation-focus?18:08
Ursinha-nomEdwinGrubbs: yes, please18:09
=== Ursinha-nom is now known as Ursinha
EdwinGrubbsrockstar: fyi, your upgrade-ui branch shows zero diff lines on the +activereviews page.18:10
rockstarEdwinGrubbs, yes, this is why I put the real diff in the comment.18:10
rockstarEdwinGrubbs, and if you didn't see the comment in the prereq branch, you've already reviewed all the code in the upgrade-ui branch.18:11
rockstarEdwinGrubbs, it might be some artifact of the pipeline.  I'm not sure.18:11
rockstarEdwinGrubbs, howgoesit?18:29
EdwinGrubbsrockstar: I'm working on Ursinha's branch.18:29
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: Ursinha || queue [rockstar, rockstar, rockstar, rockstar,adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
rockstarEdwinGrubbs, ah, okay.18:30
rockstarEdwinGrubbs, I'll probably be heading to eat something in 30 minutes, so you may want to do the pedantry branches first (they're small).18:30
EdwinGrubbsrockstar: I'll be getting something to eat soon, too.18:31
rockstarEdwinGrubbs, great, that works out well then.18:31
rockstarI also have another branch that I'll probably need to cut in front of adiroiban with, if that's okay.18:32
adiroibanno hurry from my side :)18:32
rockstaradiroiban, great, because I kinda do have a hurry on my side.18:35
adiroibango, go, go18:35
=== rockstar changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: Ursinha || queue [rockstar, rockstar, rockstar, rockstar, rockstar, adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bacjamalta: ping18:54
jamaltabac: hey there18:55
jamaltaor rather18:55
jamaltapong :)18:55
bachi jamalta have you arranged for anyone to land your changes from yesterday?18:55
* jamalta tries to remember which changes from yesterday bac is referring to18:55
jamaltabigjools said he would land my changes from today18:55
bacjamalta: the two merge proposals i reviewed for you18:56
jamaltaas for yesterday, let me have a look18:56
jamaltabac: oh18:56
jamaltano, i didn't know i was supposed to find someone to land them18:56
bacjamalta: ah, ok.  yeah it doesn't happy automatically18:56
bacjamalta: i'll be glad to do it.18:56
jamaltabac: well i know that! what i meant is that usually whoever approved them took care of landing them18:56
bacjamalta: it's best if you can remember to either ask the reviewer or someone else18:56
jamaltabac: thanks! that would be great18:56
jamaltabac: right, well next time i'll make sure to ask :) thanks18:57
bacjamalta: i should've asked.18:57
bacif we both try to remember perhaps one will18:57
jamaltabac: haha, sounds like a deal18:57
jamaltawell thanks for pointing that out19:00
jamaltai really appreciate it19:00
EdwinGrubbsUrsinha: what information do I need to add to make a series translatable?19:00
UrsinhaEdwinGrubbs: you need to add a template so you have what to translate on it19:01
UrsinhaEdwinGrubbs: to be really honest I've tested that by adding it in the tests, with factories19:01
bacjamalta: can you go to the MPs and set the commit message.  it is the text used when we send it to PQM19:05
bachttps://code.edge.launchpad.net/~jamalta/launchpad/newtag-statement-redundancy/+merge/1767019:05
jamaltabac: sure thing19:05
jamaltasorry, forgot to do it for those19:05
jamaltabac: ok it's changed19:07
bacthx19:07
Ursinhadanilos: are you there?19:11
Ursinhaor henninge :)19:11
Ursinhahenninge: which scripts need to be run for the imports to be approved in my local instance?19:11
henningeUrsinha: AFAIK you need to make run_all19:13
henningeUrsinha: and then19:13
henningeUrsinha: uploads or bzr imports?19:13
Ursinhahenninge: uploads19:13
EdwinGrubbsUrsinha: since this came up in the reviewer meeting, I'll ask, did you have a pre-impl call with someone on the translations team.19:14
UrsinhaEdwinGrubbs: yes19:15
henningeUrsinha: you can approve manually from the queue, unless you wish to try out the gardener19:16
Ursinhahenninge: admin user isn't a rosetta admin?19:16
henningeUrsinha: and then "cronscripts/rosetta-poimport.py"19:17
Ursinhahenninge: because the Approve option is grayed19:17
henningeUrsinha: you need to set an import target first.19:17
henningeUrsinha: edit the entry19:18
henningeUrsinha: if it's a new template, make up a domain and template name19:19
Ursinhahenninge: now everything makes sense :)19:19
Ursinhahenninge: what must happen for the imports to leave the queue?19:34
Ursinhahenninge: I see they were imported here, and approved, and still on the list19:35
henningeUrsinha: the gardener will clear them out after a few days.19:35
henningeUrsinha: same for "Deleted" entries.19:35
Ursinhahenninge: I see19:35
henningeUrsinha: you do know what the gardener is?19:36
Ursinhahenninge: I presume it's a script that cleans things19:36
henningeUrsinha: it used to called the "auto approver", it approves and cleans, yes.19:37
henningeThat's why jtv renamed it.19:37
Ursinhahenninge: I see19:38
Ursinhathanks for the explanation19:41
gary_posterflacoste or gmb: review https://code.edge.launchpad.net/~gary/launchpad/bzr-builder-symlink/+merge/17774 for testfix?19:41
flacostegary_poster: r=me19:42
gary_posterflacoste: thanks19:42
Ursinhahenninge: merging caused no conflicts :)19:45
henningeUrsinha: See how smart Bazaar is? ;-)19:45
Ursinhahenninge: hehehe :)19:45
bacjamalta: your branches have been sent off to ec2.  they will certainly fail when submitted to PQM due to buildbot being in testfix but i'll land them later.19:52
flacostebac: testfix mode should be off19:54
flacostei just reviewed gary's fix19:54
bacflacoste: so it'll be a foot race19:55
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: Edwin || reviewing: rockstar || queue [rockstar, rockstar, rockstar, rockstar, adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bacflacoste: and, sadly, i have a head start19:55
flacostebac: i think gary will submit his branch directly to PQM19:55
flacosteso it should land before yours EC2 tests complete19:55
gary_posterflacoste, bac, yes19:55
gary_posterbac, it is in PQM now19:56
bacflacoste: ah, right, so we go out of testfix as soon as buildbot starts working on it, not finishing?19:56
flacostebac: yes19:56
bacexcellent19:57
flacostebac: as soon as PQM sees a testfix revision, it considers that the tree is clean19:57
flacosteoptimist algorithm19:57
gary_posterfrom flacoste, the optimist :-)19:57
jamaltabac: ah ok20:02
jamaltathanks for everything :)20:02
bacjamalta: np.  you did all the hard work.20:10
=== salgado is now known as salgado-afk
jamaltabac: well, i really appreciate the help and insight :)20:12
jamaltait's really helping me learn launchpad20:13
=== mwhudson_ is now known as mwhudson
bacadiroiban: thanks for the fix.  i've approved the MP with a two-line change20:24
adiroibanbac: ok. will you send the branch for testing ? or should I run the full tests?20:25
bacadiroiban: i will land it for you20:25
bacadiroiban: just let me know when it is finalized20:25
adiroibanbac: finalized?20:25
bacadiroiban: also, i note some of the changes you made introduced trailing whitespace.  if you can remove those it'd be great.20:26
bacadiroiban: yes, i make one  final code change suggestion20:26
bacadiroiban: sorry if i'm being unclear20:26
adiroibanbac: ah. got the email :)20:26
bacok20:26
adiroibana bit of delay20:26
adiroibanok20:26
=== matsubara is now known as matsubara-afk
adiroibanbac: done. I was thinking that „make lint” can catch such errors20:37
bacadiroiban: so you have pushed your changes and want me to land it as is?20:39
adiroibanbac: what are the outstanding problems?20:39
bacadiroiban: none.  i just got your diff and it looks good20:40
bacadiroiban: so i will land it as is.  thanks.20:40
adiroibanthanks for your help and guidance! much appreciated20:41
rockstarEdwinGrubbs, hi.  Do you have any questions so far?20:54
EdwinGrubbsrockstar: not yet. sorry I've been slow starting on it.20:54
rockstarEdwinGrubbs, that's okay, as long as you can get to the branch upgrade ones today.  The pedantry branches can probably get done tomorrow.20:55
EdwinGrubbsrockstar: so, is BZR_BRANCH_8 the same as 2a?20:57
rockstarEdwinGrubbs, no, BZR_BRANCH_8 is a branch format.  2a is a repository format.20:58
rockstarEdwinGrubbs, in a follow up branch, I add BZR_BRANCH_7 to the non-upgradables as well, since BZR_BRANCH_8 is still considered a development format.20:58
rockstarEdwinGrubbs, also, all these branches will probably land in one final branch together (hopefully today)21:02
EdwinGrubbsrockstar: I see where BranchUpgradeJob.create() checks upgrade_pending, but I don't see where that attribute is set. Is that in another branch?21:14
rockstarEdwinGrubbs, which branch are you looking at?21:15
EdwinGrubbsrockstar: upgrade-job-2.0-changes21:15
rockstarEdwinGrubbs, it's a property defined in the model code.21:16
EdwinGrubbsrockstar: Shouldn't lp/code/configure.zcml specify that requestUpgrade() requires launchpad.Edit instead of launchpad.View?21:22
rockstarEdwinGrubbs, yes, you are right.21:22
rockstarEdwinGrubbs, also, I'd like to expose that through the API eventually, but exposing jobs in the API right now is a little awkward.21:23
leonardrgary: given edwin's queue, would you like to review another of my branches?21:25
leonardrdo you have time today?21:25
gary_posterleonardr: I'll give it a whirl, sure21:26
EdwinGrubbsrockstar: review sent21:27
rockstarEdwinGrubbs, cheers! One down, five to go.21:28
rockstarEdwinGrubbs, I can take adiroiban's review if you'd like.21:28
EdwinGrubbsrockstar: you make it sound like a drinking game.21:28
EdwinGrubbsrockstar: I would appreciate it if you take his mp21:28
rockstarEdwinGrubbs, only when it's Soyuz branches.21:28
rockstaradiroiban, ping21:31
adiroibanrockstar: hi21:31
rockstaradiroiban, why the change to lib/lp/registry/browser/peoplemerge.py21:31
rockstarOr rather, any of the people-merge stuff.21:32
adiroibanI messed up something 21:32
rockstaradiroiban, actually, most of this branch looks unrelated to the bug you reference.21:32
adiroibanhm...somethings get messed durring the devel merge21:34
rockstaradiroiban, you submitted the merge against db-devel21:36
adiroibanah...21:37
adiroibantrue21:37
adiroibancan I change the MP?21:37
adiroibanor should I create a new one for lp-devel?21:37
leonardrgary: https://code.edge.launchpad.net/~leonardr/lazr.restful/operation-removed-in/+merge/1778421:38
gary_posterleonardr: ack, looking21:38
adiroibanrockstar: sorry, here is the MP https://code.edge.launchpad.net/~adiroiban/launchpad/bug-359180-take-2/+merge/1778521:39
rockstaradiroiban, could you please delete the old one then?21:39
gary_posterleonardr: typo versuibed21:39
adiroibandone21:39
leonardrgary: i fixed that but haven't pushed yet21:40
gary_posterleonardr: cool21:40
adiroibanthe new diff looks ok21:40
rockstaradiroiban, yes, this looks much better.21:40
rockstaradiroiban, I'm assuming you talked to someone in translations about this?21:40
adiroibanrockstar: yes. the big discussion is on the but comments21:41
adiroibanand this is also the second part of the change21:41
rockstaradiroiban, line 104 of the patch needs force returns in it.  It is too long.21:42
gary_posterleonardr: in @call_with(fixed=...), I am not familiar with that kwarg feature.  That means that, when the webservice calls the method, it includes the value as a kwarg (and does not allow a webservice caller to override it)?21:43
adiroibanrockstar: yes. I don't know why make lint did not catch that error. I'm fixing my MP script to catch such errors21:43
rockstaradiroiban, it's a pagetest, and lint doesn't look at non-python files for lint.21:43
leonardrgary: yes, that argument does not show up as part of the named operation's signature21:44
gary_posterleonardr: got it, thanks21:44
gary_posterleonardr: approved21:49
leonardrgary, great21:50
jamaltai have to admit, that's one rockstar queue21:51
rockstarjamalta, it's the best kind.21:52
jamaltarockstar: ;)21:52
EdwinGrubbsrockstar: it took me a while to find the upgrade link, since I expected it in the action menu for the branch as opposed to thinking about it as editing an attribute. Can you create a screenshot of that and ask beuno about the placement?22:02
rockstarEdwinGrubbs, well, this isn't the final UI.  I have one more branch that does all the polish, etc. but I need some graphics.22:02
rockstarEdwinGrubbs, I think the link needs to be much more prominent in these cases, since we REALLY want people to upgrade.  It's possible, however, that we'll be backing out the UI for this before next week's rollout.22:03
EdwinGrubbsrockstar: yeah, it also seems you would want to encourage users to press the upgrade button but also warn them about what they are about to do on the form.22:03
rockstarEdwinGrubbs, basically, I want a big button, but big buttons are a no-no now, apparently.  I'm not sure how it's going to happen, but I'm going to leave that until we know the rest of the feature is okay.22:04
rockstarThe plan is to spend today and tomorrow QAing.  If the actual process is good, then the new UI will land as soon as PQM opens back up.22:05
rockstar(merely because UI work is not release critical)22:05
=== mwhudson_ is now known as mwhudson
bacadiroiban: could you set the commit message on https://code.edge.launchpad.net/~adiroiban/launchpad/bug-340662-take-2/+merge/1759822:24
adiroibanbac: done22:28
bacadiroiban: thanks22:28
gary_posterflacoste: are you available for a pretty quick review of the hack fix for bug 491705, https://code.edge.launchpad.net/~gary/launchpad/bug-491705-hack/+merge/17788 ?22:29
mupBug #491705: AttributeError: 'NoneType' object has no attribute 'utf_8_decode' <Launchpad Foundations:In Progress by gary> <https://launchpad.net/bugs/491705>22:29
flacostegary_poster: was actually looking at it right now :-)22:29
gary_posterflacoste: :-) thanks22:29
adiroibanbac: was the import warning solved on devel?22:30
adiroibanfor IProduct22:30
bacyes22:30
adiroibanok22:31
adiroibanthere is one more thing regarding translations tests.22:32
adiroibanit looks like they are launched twice22:32
adiroibanTests with failures: lib/lp/translations/browser/tests/pofile-views.txt lib/lp/translations/tests/../browser/tests/pofile-views.txt22:32
bacadiroiban: odd, i haven't seen that before22:37
bacadiroiban: are those tests failing now?22:37
EdwinGrubbsrockstar: when registering a new branch, I only see Hosted, Mirrored, and Remote branch types. How does an Imported branch get added?22:37
rockstarEdwinGrubbs, there's a different flow for import branches.  It's on a link that says "Import a branch"22:37
rockstarEdwinGrubbs, gnome-terminal/imported is a sample import branch.22:38
EdwinGrubbsrockstar: is there any reason that a branch such as ~vcs-imports/evolution/main, which has an Imported branch type, but hasn't been imported yet, should not show that link?22:40
rockstarNo, I don't think so.22:40
rockstarEdwinGrubbs, unless you don't have permission to see the link.22:40
rockstarEdwinGrubbs, your best bet is to log in as mark.22:41
adiroibanrockstar: I've fixed all long lines from that file.22:42
rockstaradiroiban, great, thanks.22:42
adiroibanbac: ah.. the test are not failing. Is just they are run twice22:42
adiroibanbac: for testing, I changed a test so that it will fail... and I can see the test is run twice22:43
bacadiroiban: that's interesting but unrelated to your branch.  perhaps you could open a bug22:45
EdwinGrubbsrockstar: it's apparently caused by the <tal:has-codeimport condition="branch/code_import"> conditional in branch-import-details.pt. Do you think it would be ok to move it outside that condition?22:46
rockstarEdwinGrubbs, I don't think.  An import branch shouldn't ever exist without a code_import as well.22:47
rockstarI just fixed a bug pertaining to that.22:47
rockstarEdwinGrubbs, so in this case, the sample data sucks.22:47
=== mwhudson_ is now known as mwhudson
EdwinGrubbsrockstar: ok, sounds good. r=me22:47
rockstarEdwinGrubbs, great, thanks.22:47
EdwinGrubbsrockstar: do you want me to review pedantry3 or jam-suggested-changes next?22:48
rockstarEdwinGrubbs, probably the one from jam next, please.22:48
adiroibanbac: bug filled22:54
adiroibanrockstar: do I need to set the commit message for this branch ?22:55
bacadiroiban: great.  your branch is off at ec2 running.  i'll bounce you the email when it comes back.22:55
rockstaradiroiban, do you need your that branch landed?22:55
=== flacoste is now known as flacoste_afk
adiroibanrockstar: what do you mean? :)22:55
adiroibanshould I found someone else to land it?22:56
rockstaradiroiban, my answer to your question is in direct relation to the answer to my question.  If you want it landed, then yes, please set the commit message.22:56
adiroibanrockstar: :) done :)22:59
EdwinGrubbsrockstar: I sent the review. I will be afk for a little bit, but I'll be able to look at your reply and the last branch today.23:12
=== EdwinGrubbs is now known as Edwin-afk
EdwinGrubbsrockstar: I'm back23:48
rockstarEdwinGrubbs, looking at your comments now.23:53
EdwinGrubbsrockstar: btw, I just sent the other review.23:55
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on-call: - || reviewing: rockstar || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews

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