[01:50] <jtv> come on you slackers, I want a review!
=== jtv changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [jtv] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[03:53] <thumper> mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/hg-imports-using-url/+merge/17187
[03:53] <mwhudson> thumper: i'm writing billions of comments for my db patch
[03:53] <thumper> heh
[03:53] <thumper> I'm about to go through your last respones
[03:53] <mwhudson> thumper: also there are no requested reviews, how did you do that?
[03:54] <thumper> mwhudson: it is in progress :)
[03:54] <mwhudson> ah
[03:54] <thumper> stub: https://code.edge.launchpad.net/~thumper/launchpad/imports-urls/+merge/17172
[03:54] <thumper> stub: I took jelmer's import work
[03:54] <thumper> stub: and unified the url fields
[03:54] <thumper> stub: in prelim of the hg import work
[04:00] <stub> mwhudson: SourcePackageRecipeDataInstruction.recipe_data is still nullable
[04:00] <mwhudson> stub: darn
[04:01]  * mwhudson pushes again
[04:10] <stub> mwhudson: Looks like the owner's name is used to refer to recipes. Is this going to explode if someone changes their Person.name?
[04:11] <mwhudson> stub: recipes don't refer to other recipes (yet)
[04:11] <mwhudson> stub: i guess it will explode much like branches
[04:11] <stub> mwhudson: And this only exists in Launchpad? Or are the URLs or ids referenced in sourcepackages or anything like that?
[04:12] <mwhudson> stub: hm
[04:12] <mwhudson> stub: actually, all references within launchpad will be at the database level
[04:12] <mwhudson> URLs won't be stored by programs outside launchpad, i expect
[04:12] <stub> I never got a rationale on the SourcepackageRecipe/SourcepackageRecipeData split either.
[04:13] <stub> Unless you need to keep orphaned SourcepackageRecipeData information I can't see the gain.
[04:14] <mwhudson> i've explained this several times
[04:14] <mwhudson> although possibly not in an email to you specifically
[04:14] <mwhudson> SourcepackageRecipeData is also referenced from SourcePackageRecipeBuild
[04:14] <stub> Not to me or in the MP where I asked the q ;)
[04:14] <mwhudson> stub: one way of looking at the split is that the SourcepackageRecipeData is the bzr-builder part
[04:15] <mwhudson> stub: and SourcePackageRecipe is the launchpad metadata around it
[04:15] <mwhudson> a little like the way the bazaar branch is stored separately from the Branch row
[04:16] <mwhudson> and the reason they're separate is that the build needs to refer to the manifest, which is a "frozen" version of the bzr-builder part of the recipe
[04:17] <stub> So the SourcePackageRecipeBuild.manifest may point to a different recipe via SPRBD than SPRB.recipe?
[04:18] <stub> Or in that case, will the manifest point to a SPRBD with no corresponding recipe?
[04:19] <mwhudson> stub: a particular SPRD is references from exactly one other row
[04:19] <mwhudson> either a SPR or a SPRB
[04:19] <mwhudson> never both
[04:20] <stub> If the pointers went the other way (add SPRD.recipe and SPRD.recipe_build), we can enforce that constraint.
[04:21] <mwhudson> stub: the patch has comments now
[04:21] <stub> Rather than leaving it to your documentation :)
[04:22] <rockstar> jtv, https://code.edge.launchpad.net/~rockstar/launchpad/use-suspended-job-status/+merge/17191
[04:22] <jtv> ahh, there it is
[04:22] <mwhudson> stub: yeah, that's probably a good idea
[04:22] <stub> mwhudson: Is bzr-builder a Launchpad project or an Ubuntu project btw?
[04:22] <mwhudson> stub: it's a james_w project
[04:22] <mwhudson> stub: i guess that makes it more an ubuntu thing than a launchpad thing
[04:24] <stub> Just wondering if revspec should be used or not. If it is the term used in the domain it is probably valid. I suspect it violates our Python coding standards though.
[04:25] <thumper> rockstar: https://code.edge.launchpad.net/~thumper/launchpad/js-play/+merge/17193
[04:25] <thumper> stub: I'd love to get https://code.edge.launchpad.net/~thumper/launchpad/imports-urls/+merge/17172 landed
[04:26] <mwhudson> stub: hm, it's a pretty standard term in the bzr world
[04:26] <thumper> :)
[04:26] <stub> mwhudson: Thats fine then.
[04:26] <mwhudson> thumper: stop distracting the DBA
[04:26] <mwhudson> thumper: this branch blocks everything
[04:26] <thumper> ok
[04:26] <thumper> mwhudson: didn't realise he was still hastling you so much
[04:27] <thumper> how about revision specifier?
[04:27] <stub> I suspect it is a good idea that this isn't entwined with the existing Build table
[04:28] <stub> If I was going to call it anything, I'd just call it 'revision' but domain naming trumps since that pulls in more meaning.
[04:30] <mwhudson> stub: for the constraints, you mean something like http://pastebin.ubuntu.com/355335/ ?
[04:31] <mwhudson> stub: revision would definitely not be right, that would imply to me that it specifies a particular revision
[04:31] <mwhudson> whereas it could be a tag, or a revno, or even a lca with some other branch
[04:32] <mwhudson> (this won't work as i have to reorder the file, but the idea is there at least)
[04:32] <stub> mwhudson: That allows them both to be NOT NULL. You want (recipe IS NULL != build IS NULL) if its one or the other but not both.
[04:33] <mwhudson> stub: doh
[04:47] <stub> mwhudson: Review in including indexes etc.
[04:48] <mwhudson> stub: thanks
[04:48] <stub> mwhudson: sourcepackage is one word in Launchpad at the moment so there is some renaming needed or discussion
[04:48] <mwhudson> stub: why is sourcepackagerecipe__owner__idx commented out
[04:48] <mwhudson> ?
[04:48] <mwhudson> stub: ok, that's easy enough to fix
[04:48] <stub> Oh - we don't need it (the unique constraint on owner, distrorelease, ... can be used.
[04:49] <mwhudson> ah ok
[04:49] <stub> Unless we see people merge timeouts because the table is too big and the index too wide ;)
[04:51] <mwhudson> stub: that definitely falls into your area :-)
[04:56] <stub> thumper: Any particular reason you are not creating a modified version of absolute_url() in database/schema/trusted.sql and fixing Bug #506146?
[04:56] <mup> Bug #506146: git and hg imports don't check for valid_absolute_url <code-import> <testing> <Launchpad Bazaar Integration:Triaged> <https://launchpad.net/bugs/506146>
[04:57] <thumper> stub: because it has to do with how we are testing git imports
[04:57] <thumper> stub: currently it is using the local file system
[04:57] <thumper> stub: and I don't want to fix this in this branch
[04:57] <thumper> stub: we'd need to fire up a git server in order to fix this bug
[04:57] <thumper> stub: so we'll probably add git and hg to the developer dependancies
[04:58]  * thumper leaves now
[04:58] <stub> thumper: The web ui protects us?
[04:58] <thumper> stub: it does
[05:01] <stub> mwhudson: I'm out to lunch. You can land the branch with the fixes if it is blocking things - I can do any tweaks post landing if they are needed.
[05:02] <mwhudson> stub: we're EODing now, so i'll make fixes, push and if you say ok i'll land it later
=== stub1 is now known as stub
[05:35] <al-maisan> noodles775: https://code.edge.launchpad.net/~al-maisan/launchpad/url-props-505382/+merge/17197
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb (reduced workload whilst fixing checkwatches) || reviewing: - || queue [jtv] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[11:14] <stub> mwhudson: Should BuildSourcePackageFromRecipeJob.job be UNIQUE?
[11:14] <mwhudson> yes, i guess
[11:15] <mwhudson> mind you it seems branchjob.job should also be unique and it isn't
=== matsubara-afk is now known as matsubara
[11:23] <stub> mwhudson: Feel free to fix branchjob.job in your patch while you are there :)
[11:23] <stub> More indexes I missed the first time sorry and a constraint fix.
[11:56] <mwhudson> stub: ok, now?
[12:00]  * stub looks
[12:02] <stub> quick, land it before I notice anything else.
[12:03] <mwhudson> thanks
[12:05] <mwhudson> :)
[12:05]  * mwhudson zzzs
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac (gmb:reduced workload whilst fixing checkwatches) || reviewing: -,- || queue [jtv] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[12:24] <bac> good morning gmb
[12:25] <gmb> bac: Hi bac.
[12:25] <bac> gmb: pretty quiet around here today?
[12:25] <gmb> bac: Yes, so far.
[12:25] <bac> cool.  have fun with CW.  i'll grab jtv's branch now
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac (gmb:reduced workload whilst fixing checkwatches) || reviewing: -,jtv || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[12:41] <bac> jtv: ping
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac (gmb:reduced workload whilst fixing checkwatches) || reviewing: -,- || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,- || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,sinzui-w&w || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== mrevell is now known as mrevell-lunch
[13:52] <bac> beuno: any thoughts on the new screenshots i sent you?
[13:54] <beuno> bac, looking now (was knee deep in some sprite fixes)
[13:55] <beuno> bac, looks much much better, thanks
[13:55] <beuno> 2 quick questions
[13:55] <beuno> where does the "continue" link take you?
[13:58] <beuno> actually just that question  :)
[13:59] <beuno> bac, also, I have an almost trivial branch for review when you have a minute: https://code.edge.launchpad.net/~beuno/launchpad/sprite-issues-never-die/+merge/17229
[14:02] <bac> beuno: 'continue' takes you back to ~beuno
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,sinzui-w&w || queue [beuno] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[14:03] <beuno> bac, s/continue/Back to my account/ and ui=me
[14:03] <bac> beuno: great, thanks
[14:03] <bac> beuno: can you update the MP so i can use 'ec2 land'?
[14:04] <beuno> bac, I can, do you have the link handy?
[14:04] <bac> https://code.edge.launchpad.net/~bac/launchpad/bug-419930/+merge/17015
[14:05]  * beuno goes get his rubber stamp
[14:07] <beuno> bac, done
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,beuno || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[14:24] <bac> beuno: what about this in projects-index.pt: <p class="sprite error application-summary"
[14:24] <bac> beuno: does it need changing too?
[14:30]  * beuno looks
[14:30] <beuno> bac, yes, it's the only place where that sprite is used
[14:30] <beuno> since I renamed it, I need to change it as well
[14:31] <bac> beuno: ok, r=bac with that.  MP updated
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,- || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[14:33] <beuno> bac, thanks
[14:33] <gmb> henninge: Re: my review from yesterday. W.R.T the use of constants and updating doctests, I think it would also be a good idea to make the tests for _getHeatFrom*() into unittests, since we'll want to add more later and unittests are more flexible in that way. How does that sound to you?
[14:34] <gmb> (I'm using constants as you suggested, though)
[14:34] <henninge> gmb: I like unittests ;-)
[14:34] <gmb> henninge: Ah, good! I shall do that forthwith, then.
[14:35] <henninge> gmb: and you are right, the way they are multiplied there for coverage does not belong in a doctest.
[14:35] <henninge> and good coverage is cool.
[14:35] <gmb> henninge: Right. I wrote the doctest because I was doing TDD and trying to work out the best way to do things, but now that I've done that a unittest would be better to ensure good coverage.
[14:43] <henninge> gmb: I can't wait to see the new code ... ;_)
=== matsubara is now known as matsubara-lunch
[15:06]  * beuno curses at ec2 test not being headless by default
=== salgado is now known as salgado-lunch
=== mrevell-lunch is now known as mrevell
[15:43] <bac> salgado-lunch: you asked that i file a bug in your review of https://code.edge.launchpad.net/~bac/launchpad/bug-419930/+merge/17015 -- i've discussed the issue at length with edwin and he says it is handled.  so i'd rather not open a bug unless you insist.
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,sinzui || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[16:11] <gmb> henninge: I don't think the BugHeatConstants need to be in interfaces/bug.py. They're only going to be used in the one place.
[16:12] <henninge> gmb: what about the rule that tests must not import from model code?
[16:12] <henninge> At least I thought there was such a rule ...
[16:12] <gmb> henninge: We aren't importing from model code though. We're importing from scripts.bugheat, which has nothing to do with the model whatever.
[16:13] <gmb> henninge: I'm not familiar with that rule, though it makes sense (We should be using the factory where possible, getUtility() where not)
[16:13] <henninge> gmb: ok, I am not too fussed about that.
[16:13] <gmb> Cool.
[16:13] <henninge> exactly
=== salgado-lunch is now known as salgado
[16:16] <salgado> bac, don't worry about it, then.
[16:17] <bac> salgado: great
=== beuno is now known as beuno-lunch
=== matsubara-lunch is now known as matsubara
[16:52] <bac> hi sinzui, i'm looking at https://code.edge.launchpad.net/~sinzui/launchpad/non-existent-packages-bug-204119/+merge/17176
[16:52] <bac> sinzui: at line 64 is there a reason you're using PackagingAddView in the super() and not the actual class name?
[16:53] <sinzui> oops
[16:54] <sinzui> bac: Yes, I am incompetent. I moved the method from  PackagingAddView when I decided it was not the right place for the rule.
[16:54] <bac> sinzui: easy mistake
[16:55]  * bac hates super() syntax anyway
[16:55] <sinzui> bac: The code works because PackagingAddView does not have a setupFields.
[16:55] <sinzui> I will fix that
[16:59] <mrevell> henninge_, If you're still around, I've updated my translations help MP -- https://code.edge.launchpad.net/~matthew.revell/launchpad/translations-help-10.1/+merge/17032
=== henninge_ is now known as henninge
[17:00] <henninge> mrevell: I am, I will look at it in a minute.
[17:00] <mrevell> thanks henninge
=== bac changed the topic of #launchpad-reviews to: on-call: gmb, bac || reviewing: lunch,/unch/sinzui || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[17:55] <henninge> gmb, bac: If either of you feels like reviewing my branch, I'd be really happy.
[17:56] <henninge> It is oversized and I won't be around until tomorrow, so I'd understand if you refuse. https://code.edge.launchpad.net/~henninge/launchpad/bug-503454-security-py/+merge/17241
[17:56] <henninge> gmb is probably done for the day anyway.
[17:58] <henninge> mrevell: I like the mo-style layout comment ;)
=== beuno-lunch is now known as beuno
[17:58] <mrevell> heh
[17:59] <mrevell> I'm heading off to cook. Will be back around these parts three or four hours.
=== mrevell is now known as mrevell-dinner
=== salgado is now known as salgado-afk
=== matsubara is now known as matsubara-afk
=== bac changed the topic of #launchpad-reviews to: on-call: bac || reviewing: henning|| queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
[21:45] <flacoste> sinzui: i replied to your review
[21:45] <sinzui> great because I just sent my windmill test fixes to EdwinGrubbs. I'll take a look at your changes
=== Ursinha is now known as Ursinha-afk
[22:17] <kfogel> bigjools: see my mail -- need a 100x100 px face or avatar image from you for planet.launchpad.net
[22:18] <bigjools> kfogel: feel free to use my LP mug
[22:19] <kfogel> bigjools: thx
[22:21] <kfogel> bigjools: the cartoony one? :-)
[22:26] <bigjools> kfogel: yep :)
[22:27] <bigjools> kfogel: it's actually a scan of what was on my wedding invitations
[22:28] <kfogel> bigjools: I also just used it as a test attachment on a bug in my dev instance, for working on bug #506018.
[22:28] <kfogel> so there
[22:28] <bigjools> kfogel: copyright infringement!
[22:28] <mup> Bug #506018: Need a "+patches" view: report lists patches attached to bugs. <story-patch-report> <Launchpad Bugs:Triaged by kfogel> <https://launchpad.net/bugs/506018>
[22:28] <kfogel> bigjools: no, because I'm not publishing :-)
[22:29] <bigjools> :)
[22:29] <kfogel> bigjools: btw, one's launchpad.dev DB stays unmolested between 'make run', shutdown, 'make run', shutdown, etc, right?  I won't lose this new project when I shut down, only when I do 'make schema', I'm guessing?
[22:30] <bigjools> kfogel: yep, only make schema blitzes it
[22:31] <kfogel> bigjools: cool
[22:31] <kfogel> bigjools: I can't quite tell when I need to restart.  Apparently, not after modifying a .pt file?  But always after modifying a .py file?  (I mean to see the effect of my changes.)
[22:32] <bigjools> kfogel: that is correct on both counts
[22:37] <EdwinGrubbs> abentley: are you still on call?
[22:38] <abentley> EdwinGrubbs: No, I'm at a sprint.
[22:38] <EdwinGrubbs> ok