=== 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
[08:08] <jtv> noodles775: 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:10] <noodles775> jtv: yeah, I saw all the improvements you did (sorry I didn't reply - I left for the day and forgot the next). Thanks!
[08:11] <jtv> noodles775: can I chuck my branches on the queue?  Got 2, and about to write another MP
[08:11] <noodles775> jtv: please do :)
[08:11] <jtv> noodles775: 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.  ;-)
=== 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
[08:13] <noodles775> jtv: 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!
=== 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
[08:14] <jtv> noodles775: <grin>
[08:14] <noodles775> Yay, my partner in crime!
[08:15] <al-maisan> noodles775: 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] <noodles775> al-maisan: yup, thanks!
=== 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
[08:22] <al-maisan> noodles775: in line 23 of the diff: why the "not None" check for tokens?
[08:24] <noodles775> al-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] <noodles775> Hrm.
[08:24]  * noodles775 thinks some more.
[08:25] <al-maisan> when are subscriptions without tokens used at all?
[08:25]  * al-maisan thought a subscription always has a token..
[08:25] <noodles775> al-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:26] <al-maisan> noodles775: how is an attempt to access a p3a noticed or pereceived?
[08:27] <noodles775> al-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-maisan> I see
[08:28] <al-maisan> so, a subscription without a token is one where no token has been generated for a team member yet .. is that right?
[08:28] <noodles775> al-maisan: yes, that's right.
[08:28] <noodles775> The tokens must be individual for security reasons, where as the subscription is not.
[08:29] <noodles775> s/not/not necessarily
[08:30] <noodles775> al-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] <noodles775> That way I would not need the check for token being None.
[08:30] <al-maisan> noodles775: great, thanks!
[08:35] <noodles775> al-maisan: pushed 10429, test unchanged.
[08:47]  * al-maisan looks
[09:00] <al-maisan> noodles775: r=me
[09:01] <noodles775> Thanks al-maisan
=== 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
[09:03] <al-maisan> jtv: looking at this mp now: https://code.edge.launchpad.net/~jtv/launchpad/bug-327575/+merge/20591
[09:04] <noodles775> Hi 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/20592
[09:04] <jtv> al-maisan: I can only encourage that
[09:04] <noodles775> adeuring: do you know if there's a reason why linked_branches is declared both in IBug and IHasLinkedBranches (which is inherited by IBug)?
[09:05] <jtv> al-maisan: can you claim the review in advance so noodles775 doesn't pick up the same one?
=== 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
[09:05] <adeuring> noodles775: more or less. (1) class IBugBranch defines a few more properties, like revision_hint or bug_task
[09:06] <adeuring> erm that was it, no (2)
[09:06] <al-maisan> jtv: review claimed
[09:07] <al-maisan> noodles775: I am on https://code.edge.launchpad.net/~jtv/launchpad/bug-327575/+merge/20591
[09:07] <jtv> thanks
[09:07] <adeuring> noodles775: 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:08] <noodles775> adeuring: sorry, I don't understand what that has to do with IBug.linked_branches and IHasLinkedBranches.linked_branches, when IBug inherits IHasLinkedBranches
[09:08] <noodles775> adeuring: oh, ok.
[09:08] <adeuring> noodles775: yes, that's quite convoluted ;)
[09:09] <noodles775> adeuring: 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] <adeuring> noodles775: and: Brian should use IBugBranch as the reference type
[09:10] <adeuring> noodles775: sorry wrote the last line too early...
[09:10] <adeuring> we need the new declaration at least now, for the export, because of  schema=IBranch, which is wrong
[09:11] <adeuring> it should be schema=IBugBranch, in the export declaration
[09:11] <noodles775> Ah, so it's actually meant to override the IHasLinkedBranches declaration?
[09:12] <noodles775> (as it uses the wrong schema). OK.
[09:12] <adeuring> noodles775: well, I did not write the code ;) But I assume, yes
[09:12] <noodles775> adeuring: ok, thanks for the help!
[09:16] <adeuring> noodles775: 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:17] <noodles775> adeuring: yes, afaics IHasLinkedBranches is used by other objects (blueprints).
[09:17] <adeuring> right
[09:37] <bigjools> wgrant: did you check the latest diff of that branch?  I added more tests and stuff
[09:40] <wgrant> bigjools: I checked the diff on the MP.
[09:40]  * wgrant checks again.
[09:41] <bigjools> wgrant: I deleted that MP and did a new one
[09:41] <bigjools> unless you looked at the new one?
[09:41] <wgrant> I 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:42] <bigjools> wgrant: :/
[09:43] <wgrant> bigjools: I don't see tests for the two things I mentioned in the latest version of the branch.
[09:47] <bigjools> wgrant: test for those are there...?
[09:47] <bigjools> oh a positive test
[09:47] <bigjools> meh
[09:48] <bigjools> good point
[09:48] <bigjools> I was lost in tests
[09:48] <wgrant> One positive, one negative.
[09:48] <bigjools> although the second one is tested
[09:48] <wgrant> There are an awful lot of tests.
[09:48] <wgrant> Is it?
[09:48] <bigjools> yeah
[09:48]  * wgrant looks harder.
[09:48] <bigjools> look for /etc/passwd
[09:49] <wgrant> That's a target.
[09:49] <bigjools> meh^2
[10:06] <noodles775> jtv: 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:10] <noodles775> hrm, 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:12] <noodles775> jtv: nm, works as expected if the login call is *before* your _simulatReadOnlyMode() call. r=me.
[10:13] <jtv> noodles775: great, thanks!
[10:14] <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] <noodles775> Yep.
=== 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
[10:40] <al-maisan> jtv: can there be only one translator per language in a given translation group?
[10:40] <jtv> al-maisan: yes, enforced by the db
[10:40] <al-maisan> great, thanks!
[10:40] <henninge> al-maisan: but it's usually a team
[10:40] <al-maisan> I see.
[10:41] <jtv> exactly
[10:55] <al-maisan> jtv: the sorting inside the `translator_list` property is not necessary any more because it is done by the fetchTranslatorData() method. Right?
[10:56] <jtv> al-maisan: right
[10:56] <jtv> it's documented
[10:56] <jtv> (otherwise it'd be evil to rely on)
[10:56] <al-maisan> jtv: nice branch, r=me
[10:56] <jtv> al-maisan: great, hvala!
[10:56] <jtv> I mean, danke.  :)
[10:56] <al-maisan> :)
[10:58] <jtv> noodles775, al-maisan: I know you've done one each already, but I do have another one
[10:58] <jtv> could I..?
[10:58] <al-maisan> jtv: sure :)
[10:58] <jtv> thanks :)
=== 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
[10:59] <jtv> heh
[10:59] <al-maisan> yeah .. we are starting to keep a tally ;P
[11:02] <jtv> al-maisan: the kanban board's good for that...  I'm piling up a nice backlog for when PQM opens.
[11:02] <al-maisan> right.
[11:03] <al-maisan> jtv #3 == https://code.edge.launchpad.net/~jtv/launchpad/bug-520651/+merge/20632 ?
[11:04] <jtv> al-maisan: indeed!
[11:04] <al-maisan> thanks!
=== 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
[14:09] <leonardr> EdwinGrubbs, see my follow-up for https://code.edge.launchpad.net/~leonardr/lazr.restful/make-test-class-public
=== jamalta-afk is now known as jamalta
[14:16] <salgado> noodles775, al-maisan-lunch, I have a small one which will be up in a minute; can one of you guys take it?
=== 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
[14:16] <noodles775> salgado: of course, pop it in :)
[14:16] <noodles775> ah, too quick.
[14:16] <salgado> thanks. :)
[14:22] <EdwinGrubbs> leonardr: it was the simplejson version. Your suggested change to setup.py fixed it. r=me
[14:34] <leonardr> noodles775, al-maisan, can you review https://code.edge.launchpad.net/~leonardr/launchpadlib/multiversion-fix ?
=== 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
[14:52] <al-maisan> leonardr: there are 5 unused imports in src/launchpadlib/launchpad.py
[14:57] <leonardr> al-maisan: if you can paste them to me i'll remove them
[14:58] <al-maisan> leonardr: sys, webbrowser, RestfulHttp, OAuthRequest, EDGE_SERVICE_ROOT
[14:58] <leonardr> al-maisan: ok, i'm on it
[14:58] <al-maisan> vim-pyflakes for the win :)
=== matsubara is now known as matsubara-lunch
[15:01] <leonardr> al-maisan: EDGE_SERVICE_ROOT needs to be there for backwards compatibility
[15:01] <leonardr> i've removed the others
[15:02] <al-maisan> leonardr: "backwards compatibility"?
[15:02] <leonardr> al-maisan: EDGE_SERVICE_ROOT used to be in launchpad.py. now it's elsewhere
[15:02] <leonardr> i import it so that scripts that have 'from launchpad import EDGE_SERVICE_ROOT' will still work
[15:03] <al-maisan> ah, I see.
[15:11] <adeuring> noodles775, 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:12] <al-maisan> adeuring: add it to the queue please
=== 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
[15:21] <al-maisan> leonardr: is the comment on lines 63-65 still current/valid?
[15:21] <al-maisan> diff lines 63-65 that is..
[15:23] <al-maisan> leonardr: also, could you explain the change to the 'service_root' variable (diff lines 78-79) ?
[15:24] <intellectronica> adeuring: i hear you need a review. interested in an exchange?
[15:24] <adeuring> intellectronica: sure, let's do that
[15:24] <adeuring> where is your branch?
[15:24] <intellectronica> adeuring: cool, i'm just creating an mp
[15:24] <adeuring> ok
=== 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
[15:28] <intellectronica> adeuring: https://code.edge.launchpad.net/~intellectronica/launchpad/max-heat-from-context/+merge/20661
[15:29] <leonardr> al-maidan: yes, the comment is still current--i just added it in this branch
[15:29] <leonardr> the change to service_root is cosmetic. it used to be the version was part of the url, now the version is separate
[15:30] <al-maisan> leonardr: the test following the comment is virtually the same like the one preceding it .. or did I miss something?
[15:30] <al-maisan> The comment talks about loading from disk and another code path
[15:31] <intellectronica> adeuring: do you have an mp?
[15:31] <adeuring> intellectronica: https://code.edge.launchpad.net/~adeuring/launchpad/bug-528788-no-searchbox-without-hot-bugs/+merge/20657
=== 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
[15:32] <leonardr> al-maisan: right
[15:32] <leonardr> we run the test twice
[15:32] <leonardr> the first time, the file is not cached on disk, and code path A runs
[15:32] <leonardr> the second time, we load the cached file, and code path B runs
[15:33] <leonardr> the bug i found was that code path B was not propagating the version argument
[15:33] <al-maisan> I see
[15:33] <al-maisan> leonardr: r=me
[15:33] <leonardr> great
=== EdwinGrubbs is now known as Edwin-afk
[15:35] <al-maisan> leonardr: maybe you could add what you said above as a comment to the test.
[15:35] <leonardr> ok, i'll take another look--it seemed clear to me
[15:36] <al-maisan> leonardr: I have no doubt that it seemed clear to *you* ;)
=== 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
[15:44] <salgado-lunch> The 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-lunch> noodles775, ^ is why my diff didn't show up in the MP, I think
[15:45] <noodles775> salgado-lunch: right, thanks.
=== matsubara-lunch is now known as matsubara
[15:53] <adeuring> intellectronica: 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:54] <intellectronica> adeuring: hmmm ... that's a good point, i think you're right
[15:54] <intellectronica> adeuring: that's beyond the scope of this branch, i think, but let's file a bug about that
[15:55] <adeuring> intellectronica: 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] <intellectronica> adeuring: r=me for your branch
[15:55] <adeuring> intellectronica: thanks!
[15:55] <intellectronica> adeuring: it's easy, but it doesn
[15:56] <intellectronica> adeuring: doesn't really have anything to do with this branch
[15:56] <adeuring> intellectronica: erm, why?
[15:57] <adeuring> after all, you changed the calulation of for Projects to include data from serieses
[15:57] <intellectronica> adeuring: which branch are you looking at?
[15:57] <adeuring> intellectronica:  lp:~intellectronica/launchpad/max-heat-from-context
[16:00] <intellectronica> adeuring: 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] <intellectronica> let me paste a diff for you just to make sure
[16:00] <adeuring> intellectronica: yes, I'm using a diff against r 9072;
[16:01] <intellectronica> adeuring: pasted the diff in the mp
[16:02] <adeuring> intellectronica: OK; so, were the SQL related changes in r 9073 already reviewed?
[16:03] <intellectronica> adeuring: yes, reviewed and landed
[16:03] <adeuring> intellectronica: Ah, OK.
[16:03] <intellectronica> i agree with your comment, i just don't want to do this now, as part of this branch
[16:04] <adeuring> intellectronica: ok, so r=me
[16:04] <intellectronica> adeuring: thanks! i'll file a bug now for the suggestion you made.
[16:05] <adeuring> intellectronica: great
=== 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]
[17:13] <abentley> rockstar, https://code.edge.launchpad.net/~abentley/launchpad/increase-lease/+merge/20668
[17:15] <abentley> rockstar, https://pastebin.canonical.com/28759/
=== 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
[19:36] <leonardr> rockstar, https://code.edge.launchpad.net/~leonardr/launchpadlib/avoid-beta-beta/+merge/20689
[19:36] <rockstar> leonardr, can I get to it in 15?
[19:36] <leonardr> rockstar, no problem
[21:04] <EdwinGrubbs> rockstar: can you review this change to launchpad-dependencies? https://code.edge.launchpad.net/~edwin-grubbs/meta-lp-deps/specify-pil-version/+merge/20699
[21:07] <rockstar> EdwinGrubbs, looks good.
[21:07] <EdwinGrubbs> thanks
=== 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