/srv/irclogs.ubuntu.com/2010/03/04/#launchpad-reviews.txt

=== jamalta-afk is now known as jamalta
=== stub1 is now known as stub
=== EdwinGrubbs changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [noodles775, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== jamalta is now known as jamalta-afk
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: bdmurray || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvnoodles775: got another couple for you, if I'm not leaning on you too heavily.  The soyuz setup branch is settled & waiting for PQM to re-open, including all your suggested improvements.08:08
noodles775jtv: yeah, I saw all the improvements you did (sorry I didn't reply - I left for the day and forgot the next). Thanks!08:10
jtvnoodles775: can I chuck my branches on the queue?  Got 2, and about to write another MP08:11
noodles775jtv: please do :)08:11
jtvnoodles775: thanks!  And really hope you'll have more creamy goodness to add to the Soyuz setup, not least to show that the effort wasn't useless.  ;-)08:11
=== jtv changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: bdmurray || queue [noodles775, jtv, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775jtv: far from useless, I'd just done the manual setup before reviewing your branch, and it was sooo much easier. It'll make it much easier for people to play with and improve soyuz!08:13
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: bdmurray, - || queue [noodles775, jtv, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvnoodles775: <grin>08:14
noodles775Yay, my partner in crime!08:14
al-maisannoodles775: is this the mp I should be looking at: https://code.edge.launchpad.net/~michael.nelson/launchpad/522517-multiple-subscriptions-displayed/+merge/20567 ?08:15
noodles775al-maisan: yup, thanks!08:15
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: bdmurray, noodles775 || queue [jtv, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisannoodles775: in line 23 of the diff: why the "not None" check for tokens?08:22
noodles775al-maisan: because multiple subscriptions without tokens may be included in the iteration. If it was added as a unique token, only the first such subscription would be displayed.08:24
noodles775Hrm.08:24
* noodles775 thinks some more.08:24
al-maisanwhen are subscriptions without tokens used at all?08:25
* al-maisan thought a subscription always has a token..08:25
noodles775al-maisan: the owner of a p3a creates a subscription for a team. Individual tokens are created only when the user tries to access them here.08:25
al-maisannoodles775: how is an attempt to access a p3a noticed or pereceived?08:26
noodles775al-maisan: currently they click on what looks like a 'view' link, but it's actually a styled button in a posted form.08:27
al-maisanI see08:27
al-maisanso, a subscription without a token is one where no token has been generated for a team member yet .. is that right?08:28
noodles775al-maisan: yes, that's right.08:28
noodles775The tokens must be individual for security reasons, where as the subscription is not.08:28
noodles775s/not/not necessarily08:29
noodles775al-maisan: I just realised after your prompting, I should really be checking there for unique_archives, not unique_tokens. I'll update and re-push.08:30
noodles775That way I would not need the check for token being None.08:30
al-maisannoodles775: great, thanks!08:30
noodles775al-maisan: pushed 10429, test unchanged.08:35
* al-maisan looks08:47
al-maisannoodles775: r=me09:00
noodles775Thanks al-maisan09:01
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: bdmurray, jtv || queue [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisanjtv: looking at this mp now: https://code.edge.launchpad.net/~jtv/launchpad/bug-327575/+merge/2059109:03
noodles775Hi adeuring, would you be able to have a look at the comment I just added to https://code.edge.launchpad.net/~brian-murray/launchpad/api-export-bug-linked-branches-take2/+merge/2059209:04
jtval-maisan: I can only encourage that09:04
noodles775adeuring: do you know if there's a reason why linked_branches is declared both in IBug and IHasLinkedBranches (which is inherited by IBug)?09:04
jtval-maisan: can you claim the review in advance so noodles775 doesn't pick up the same one?09:05
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: jtv, jtv || queue [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringnoodles775: more or less. (1) class IBugBranch defines a few more properties, like revision_hint or bug_task09:05
adeuringerm that was it, no (2)09:06
al-maisanjtv: review claimed09:06
al-maisannoodles775: I am on https://code.edge.launchpad.net/~jtv/launchpad/bug-327575/+merge/2059109:07
jtvthanks09:07
adeuringnoodles775: and the history of Brian's branch is quite complex. It was merged into a another branch, and that branch landed after Brian's changed had been reverted.09:07
noodles775adeuring: sorry, I don't understand what that has to do with IBug.linked_branches and IHasLinkedBranches.linked_branches, when IBug inherits IHasLinkedBranches09:08
noodles775adeuring: oh, ok.09:08
adeuringnoodles775: yes, that's quite convoluted ;)09:08
noodles775adeuring: the declaration should only be required on IHasLinkedBranches right? (although the test fails if you remove the one on IBug)? Or what did I miss?09:09
adeuringnoodles775: and: Brian should use IBugBranch as the reference type09:09
adeuringnoodles775: sorry wrote the last line too early...09:10
adeuringwe need the new declaration at least now, for the export, because of  schema=IBranch, which is wrong09:10
adeuringit should be schema=IBugBranch, in the export declaration09:11
noodles775Ah, so it's actually meant to override the IHasLinkedBranches declaration?09:11
noodles775(as it uses the wrong schema). OK.09:12
adeuringnoodles775: well, I did not write the code ;) But I assume, yes09:12
noodles775adeuring: ok, thanks for the help!09:12
adeuringnoodles775: an odd detail of overriding linked_branches is that there is not very much left from IHasLinkedBranches ;) But that's nothing we should nag Brain with ;)09:16
noodles775adeuring: yes, afaics IHasLinkedBranches is used by other objects (blueprints).09:17
adeuringright09:17
bigjoolswgrant: did you check the latest diff of that branch?  I added more tests and stuff09:37
wgrantbigjools: I checked the diff on the MP.09:40
* wgrant checks again.09:40
bigjoolswgrant: I deleted that MP and did a new one09:41
bigjoolsunless you looked at the new one?09:41
wgrantI looked at the new one.09:41
wgrant(I also got an email about the old one when I shouldn't have; I don't have access to production-devel, yet I got a diff from it)09:41
bigjoolswgrant: :/09:42
wgrantbigjools: I don't see tests for the two things I mentioned in the latest version of the branch.09:43
bigjoolswgrant: test for those are there...?09:47
bigjoolsoh a positive test09:47
bigjoolsmeh09:47
bigjoolsgood point09:48
bigjoolsI was lost in tests09:48
wgrantOne positive, one negative.09:48
bigjoolsalthough the second one is tested09:48
wgrantThere are an awful lot of tests.09:48
wgrantIs it?09:48
bigjoolsyeah09:48
* wgrant looks harder.09:48
bigjoolslook for /etc/passwd09:48
wgrantThat's a target.09:49
bigjoolsmeh^209:49
noodles775jtv: if I modify your test so that a user is logged in when it runs, I get an assertion error? (Must not be called when there's no translation group). Is that just related to the test data, or is it worth adding such a test to ensure a logged in user will also see the read_only message?10:06
noodles775hrm, looking at the code, it can only be that me logging in is affecting your simulated read only...10:10
* noodles775 tries reordering the login.10:10
noodles775jtv: nm, works as expected if the login call is *before* your _simulatReadOnlyMode() call. r=me.10:12
jtvnoodles775: great, thanks!10:13
jtv(yeah, I thought of the stuff I'd have to test _ideally_ and that was another reason to put the read-only check at the top)10:14
noodles775Yep.10:14
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: abentley, jtv || queue [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisanjtv: can there be only one translator per language in a given translation group?10:40
jtval-maisan: yes, enforced by the db10:40
al-maisangreat, thanks!10:40
henningeal-maisan: but it's usually a team10:40
al-maisanI see.10:40
jtvexactly10:41
al-maisanjtv: the sorting inside the `translator_list` property is not necessary any more because it is done by the fetchTranslatorData() method. Right?10:55
jtval-maisan: right10:56
jtvit's documented10:56
jtv(otherwise it'd be evil to rely on)10:56
al-maisanjtv: nice branch, r=me10:56
jtval-maisan: great, hvala!10:56
jtvI mean, danke.  :)10:56
al-maisan:)10:56
jtvnoodles775, al-maisan: I know you've done one each already, but I do have another one10:58
jtvcould I..?10:58
al-maisanjtv: sure :)10:58
jtvthanks :)10:58
=== jtv changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: abentley, - || queue [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: abentley, jtv3 || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvheh10:59
al-maisanyeah .. we are starting to keep a tally ;P10:59
jtval-maisan: the kanban board's good for that...  I'm piling up a nice backlog for when PQM opens.11:02
al-maisanright.11:02
al-maisanjtv #3 == https://code.edge.launchpad.net/~jtv/launchpad/bug-520651/+merge/20632 ?11:03
jtval-maisan: indeed!11:04
al-maisanthanks!11:04
=== daniloff is now known as danilos
=== matsubara-afk is now known as matsubara
=== al-maisan is now known as al-maisan-lunch
=== mrevell is now known as mrevell-lunch
leonardrEdwinGrubbs, see my follow-up for https://code.edge.launchpad.net/~leonardr/lazr.restful/make-test-class-public14:09
=== jamalta-afk is now known as jamalta
salgadonoodles775, al-maisan-lunch, I have a small one which will be up in a minute; can one of you guys take it?14:16
=== salgado changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: abentley, jtv3 || queue [salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan-lunch is now known as al-maisan
noodles775salgado: of course, pop it in :)14:16
noodles775ah, too quick.14:16
salgadothanks. :)14:16
EdwinGrubbsleonardr: it was the simplejson version. Your suggested change to setup.py fixed it. r=me14:22
leonardrnoodles775, al-maisan, can you review https://code.edge.launchpad.net/~leonardr/launchpadlib/multiversion-fix ?14:34
=== leonardr changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: abentley, jtv3 || queue [salgado,leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: abentley, <<coffee>> || queue [salgado,leonardr] || 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: salgado, <<coffee>> || queue [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: salgado, leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisanleonardr: there are 5 unused imports in src/launchpadlib/launchpad.py14:52
leonardral-maisan: if you can paste them to me i'll remove them14:57
al-maisanleonardr: sys, webbrowser, RestfulHttp, OAuthRequest, EDGE_SERVICE_ROOT14:58
leonardral-maisan: ok, i'm on it14:58
al-maisanvim-pyflakes for the win :)14:58
=== matsubara is now known as matsubara-lunch
leonardral-maisan: EDGE_SERVICE_ROOT needs to be there for backwards compatibility15:01
leonardri've removed the others15:01
al-maisanleonardr: "backwards compatibility"?15:02
leonardral-maisan: EDGE_SERVICE_ROOT used to be in launchpad.py. now it's elsewhere15:02
leonardri import it so that scripts that have 'from launchpad import EDGE_SERVICE_ROOT' will still work15:02
al-maisanah, I see.15:03
adeuringnoodles775, al-maisan: could one of you please review this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-528788-no-searchbox-without-hot-bugs/+merge/20657 ?15:11
al-maisanadeuring: add it to the queue please15:12
=== adeuring changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: salgado, leonardr || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisanleonardr: is the comment on lines 63-65 still current/valid?15:21
al-maisandiff lines 63-65 that is..15:21
al-maisanleonardr: also, could you explain the change to the 'service_root' variable (diff lines 78-79) ?15:23
intellectronicaadeuring: i hear you need a review. interested in an exchange?15:24
adeuringintellectronica: sure, let's do that15:24
adeuringwhere is your branch?15:24
intellectronicaadeuring: cool, i'm just creating an mp15:24
adeuringok15:24
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: barry, leonardr || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
intellectronicaadeuring: https://code.edge.launchpad.net/~intellectronica/launchpad/max-heat-from-context/+merge/2066115:28
leonardral-maidan: yes, the comment is still current--i just added it in this branch15:29
leonardrthe change to service_root is cosmetic. it used to be the version was part of the url, now the version is separate15:29
al-maisanleonardr: the test following the comment is virtually the same like the one preceding it .. or did I miss something?15:30
al-maisanThe comment talks about loading from disk and another code path15:30
intellectronicaadeuring: do you have an mp?15:31
adeuringintellectronica: https://code.edge.launchpad.net/~adeuring/launchpad/bug-528788-no-searchbox-without-hot-bugs/+merge/2065715:31
=== adeuring changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: barry, leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
leonardral-maisan: right15:32
leonardrwe run the test twice15:32
leonardrthe first time, the file is not cached on disk, and code path A runs15:32
leonardrthe second time, we load the cached file, and code path B runs15:32
leonardrthe bug i found was that code path B was not propagating the version argument15:33
al-maisanI see15:33
al-maisanleonardr: r=me15:33
leonardrgreat15:33
=== EdwinGrubbs is now known as Edwin-afk
al-maisanleonardr: maybe you could add what you said above as a comment to the test.15:35
leonardrok, i'll take another look--it seemed clear to me15:35
al-maisanleonardr: I have no doubt that it seemed clear to *you* ;)15:36
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: barry, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado is now known as salgado-lunch
salgado-lunchThe script 'mpcreationjobs' didn't run on 'loganberry' between 2010-03-04 14:07:05 and 2010-03-04 15:07:05 (last seen 2010-03-04 13:13:14.534301)15:44
salgado-lunchnoodles775, ^ is why my diff didn't show up in the MP, I think15:44
noodles775salgado-lunch: right, thanks.15:45
=== matsubara-lunch is now known as matsubara
adeuringintellectronica: generaaly, your branch looks good. but one question, about recalculateMaxBugHeat(): For IProjectGroups, shouldn't the bug heat from ProductSeries-targeted bugs be also included, like it is for the max heat of projects?15:53
intellectronicaadeuring: hmmm ... that's a good point, i think you're right15:54
intellectronicaadeuring: that's beyond the scope of this branch, i think, but let's file a bug about that15:54
adeuringintellectronica: well, I think it is easy to update ;) And aren't bugs from project serises inluded in bug listing for a project group?15:55
intellectronicaadeuring: r=me for your branch15:55
adeuringintellectronica: thanks!15:55
intellectronicaadeuring: it's easy, but it doesn15:55
intellectronicaadeuring: doesn't really have anything to do with this branch15:56
adeuringintellectronica: erm, why?15:56
adeuringafter all, you changed the calulation of for Projects to include data from serieses15:57
intellectronicaadeuring: which branch are you looking at?15:57
adeuringintellectronica:  lp:~intellectronica/launchpad/max-heat-from-context15:57
intellectronicaadeuring: something makes me think that you're looking at the wrong diff. looks like lp didn't generate one, are you looking at one you generated locally?16:00
intellectronicalet me paste a diff for you just to make sure16:00
adeuringintellectronica: yes, I'm using a diff against r 9072;16:00
intellectronicaadeuring: pasted the diff in the mp16:01
adeuringintellectronica: OK; so, were the SQL related changes in r 9073 already reviewed?16:02
intellectronicaadeuring: yes, reviewed and landed16:03
adeuringintellectronica: Ah, OK.16:03
intellectronicai agree with your comment, i just don't want to do this now, as part of this branch16:03
adeuringintellectronica: ok, so r=me16:04
intellectronicaadeuring: thanks! i'll file a bug now for the suggestion you made.16:04
adeuringintellectronica: great16:05
=== noodles775 changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado-lunch is now known as salgado
=== deryck is now known as deryck[lunch]
abentleyrockstar, https://code.edge.launchpad.net/~abentley/launchpad/increase-lease/+merge/2066817:13
abentleyrockstar, https://pastebin.canonical.com/28759/17:15
=== al-maisan 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
=== danilos is now known as daniloff
=== gary_poster is now known as gary-lunch
=== jamalta is now known as jamalta-afk
=== deryck[lunch] is now known as deryck
=== jamalta-afk is now known as jamalta
=== rockstar 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
=== Edwin-afk is now known as EdwinGrubbs
=== gary-lunch is now known as gary_poster
leonardrrockstar, https://code.edge.launchpad.net/~leonardr/launchpadlib/avoid-beta-beta/+merge/2068919:36
rockstarleonardr, can I get to it in 15?19:36
leonardrrockstar, no problem19:36
EdwinGrubbsrockstar: can you review this change to launchpad-dependencies? https://code.edge.launchpad.net/~edwin-grubbs/meta-lp-deps/specify-pil-version/+merge/2069921:04
rockstarEdwinGrubbs, looks good.21:07
EdwinGrubbsthanks21:07
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== jamalta is now known as jamalta-afk
=== rockstar 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

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