[01:53] <NCommander> anyone around?
[02:05]  * thumper hides
[03:36] <NCommander> thumper: can I tempt you for a review? :-)?
[03:37] <thumper> NCommander: not right now, I'm just about to go make dinner and come back later this evening
[03:37] <NCommander> thumper: fair enough
[03:40] <mwhudson> NCommander: i might be able to have a look at it
[03:41] <NCommander> mwhudson: cool!
[03:41] <NCommander> mwhudson: https://code.edge.launchpad.net/~mcasadevall/launchpad/raw_source_changelog_librarian - it probably needs another ec2test run but I fixed all the failures that were in the email
[03:51] <mwhudson> NCommander: did you know that there's a theoretical limit of 800 lines of diff for a review?
[03:52] <mwhudson> oh i see, a lot of the diff is sampledata changes
[03:52] <NCommander> mwhudson: no I didn't, although the wiki says to ignore the sampledata change size :-/
[04:13] <mwhudson> NCommander: it's getting a bit late for me, but does your branch store the changelog in the database or in the librarian?
[04:13] <NCommander> mwhudson: librarian. See dscfile.py changes
[04:14] <NCommander> mwhudson: findAndMoveCopyright and the self.librarian.create call
[04:14] <mwhudson> NCommander: then the sampledata changes look strange
[04:14] <mwhudson> as they seem to include big chunks of changelog
[04:15] <mwhudson> NCommander: do you need to regenerate the sampledata again?
[04:16] <NCommander> mwhudson: I see a whole lot of NULLs for the pointer to LFA. There is some changelog data stored in the database by the existing code however
[04:16] <NCommander> mwhudson: thats going to get depericated and eventually removed with a little luck
[04:17] <mwhudson> oh ok
[04:17] <mwhudson> ahh, happiness is meld on a bigger monitor
[04:17] <NCommander> mwhudson: that's the changelog_entry your probably seeing, and there are quite a few issues with its design sadly, raw source changelog will hopefully let us replace it -)
[04:17] <NCommander> meld?
[04:17] <mwhudson> NCommander: a diff tool
[04:19] <NCommander> mwhudson: I'm sorry to keep you awake :-/
[04:19] <mwhudson> aagh soyuz code it burns the eyes
[04:19] <mwhudson> NCommander: it'
[04:20] <mwhudson> s only late to be concentrating, not to be awake :)
[04:20] <NCommander> mwhudson: heh, I plan to be writing more soyuz code in the future, so make sure the brian bleach is near by
[04:20] <mwhudson> createUploadedSourcePackageRelease takes far too many arguments
[04:21] <NCommander> mwhudson: no kidding :-/
[04:23] <mwhudson> NCommander: ah right, i've finally found the interesting parts of your change
[04:46] <NCommander> mwhudson: I'll be AFK for about 15 minutes
[04:47] <mwhudson> NCommander: it'll probably be that long before you have a review, but thanks for letting me know
[05:03] <NCommander> mwhudson: back
[05:03] <mwhudson> NCommander: still going!
[05:07] <mwhudson> NCommander: review submitted
[05:12] <NCommander> mwhudson: thanks. Fixing.
[05:13] <NCommander> mwhudson: maybe yo ucan explain when a module should be in __all__
[05:13] <mwhudson> NCommander: well, maybe that's overly picky of me
[05:13] <mwhudson> NCommander: one way around is that it _needs_ to be in all if it's imported in another module
[05:14] <mwhudson> NCommander: i guess the question is, is findFile part of the 'public interface' of the module
[05:14] <mwhudson> it's a close call in this case
[05:15] <NCommander> mwhudson: I think its used in the test suite
[05:16] <NCommander> mwhudson: ugh, a few of these exist just before I didn't properly clean up with all the refactoring this branch has gone through
[05:16]  * NCommander headthunks
[05:16] <mwhudson> NCommander: that's what reviews are for :-)
[05:17] <NCommander> mwhudson: with 10MiB changelog, that was a security fix that was discovered when I was writing the code
[05:18] <NCommander> mwhudson: bigjools and I threw some numbers, and we decided 10MiB was a good value. The old value was 2GiB (hardset DB limit)
[05:18] <mwhudson> NCommander: ok
[05:18] <mwhudson> NCommander: i remember some discussion
[05:18] <mwhudson> but i remembered it being about copyright, not copyright and changelog
[05:19] <mwhudson> if i remembered wrong, that's fine :)
[05:19] <NCommander> mwhudson: well, that's from an earlier version of the branch where changelog and copyright were in the database
[05:19] <NCommander> changelog will now be in librarian instead of the DB, so I could actually drop that check
[05:20] <NCommander> mwhudson: or, well, move it to debian/copyright's finder function
[05:21] <mwhudson> NCommander: in this case i'm just asking the question
[05:21] <mwhudson> making sure you've thought it through
[05:21] <mwhudson> i don't have the answer :)
[05:24] <NCommander> mwhudson: heh :-)
[05:26] <NCommander> mwhudson: you can't set changelog_path with dsc_file's path since its a temporary directory :-/
[05:26]  * NCommander was greatly annoyed by having to make that interface change
[05:27] <mwhudson> NCommander: i mean you could say dsc_file.changelog_path = foo then call findAndMoveChangelog(dsc_file)
[05:28] <NCommander> mwhudson: ah, I could do that, although then I need to whack the testing code a bit
[05:29] <NCommander> mwhudson: (I sorta like to keep my functions self contained, but maybe just because I'm weird ...)
[05:30] <mwhudson> NCommander: maybe leave it
[05:34] <mwhudson> NCommander: i'm off to sort out dinner, i'll be around ish for another 30 mins maybe but perhaps you'll need to get someone else to review the branch again
[05:41] <NCommander> mwhudson: I'm almost done fixing most of the issues
[05:53] <NCommander> mwhudson: pushed
[05:53] <NCommander> (and resubmitted)
[05:53] <mwhudson> NCommander: you don't need to resubmit
[05:55] <NCommander> mwhudson: oh ...
[05:55]  * NCommander already did :-/
[05:55] <mwhudson> well you'll know for next time :)
[05:57] <mwhudson> NCommander: sorry have to go now, i'll review it first thing tomorrow if noone else gets to it
[05:58] <NCommander> mwhudson: fair enough
[05:59] <StevenK> NCommander: If you need to make changes to a branch, just push it, the MP is clever enough to realise the branch changed
[05:59] <NCommander> StevenK: I hit the resubmit button so mwhudson Needs Fixing goes back to Review
[06:00] <StevenK> NCommander: You can edit the MP state too ...
[06:03] <NCommander> StevenK: ah
[06:06] <StevenK> NCommander: I can edit my own MPs and set them betwen WIP, Needs Review and Merged. I'm unsure why the last one is allowed, but the first two make perfect sense.
[06:07] <NCommander> StevenK: are you in ~launchpad?
[06:07] <NCommander> StevenK: I can see Merged, but I always thought that's because I'm in ~launchpad
[06:12] <StevenK> I don't think so.
[06:13] <StevenK> Confirmed, I'm not a member of ~launchpad
[06:45] <wgrant> StevenK: What harm is there in letting people set their own MPs as Merged?
[06:45] <wgrant> It's useful if it wasn't automatically detected.
[06:46] <StevenK> Well, what happens if an MP is set to Merged before it's even reviewed?
[06:46] <wgrant> It drops off the review list.
[06:47] <StevenK> IE, it could just drop through the cracks if the submitter just sets it to Merged?
[06:47] <wgrant> Yes.
[06:47] <wgrant> But if they do that, then they're pretty silly.
[06:47] <wgrant> It's just like if they set it back to Work in Progress
[06:48] <StevenK> I've done that, which is useful
[06:48] <StevenK> "I thought I was ready, oh damn, I missed x, y and z."
[07:36] <NCommander> StevenK: file a bug?
[09:22] <gmb> stub, BjornT: Could you take a look at https://code.edge.launchpad.net/~gmb/launchpad/bugwatch-next_check-bug-544238 please? Really need to get this one landed as soon as possible.
[09:26] <stub> nag nag nag... :)
[09:31] <gmb> :)
[09:31] <gmb> stub: Thanks for the review :)
[09:34] <NCommander> stub: BTW, mind looking at the revised raw source changelog (which now uses librarian when you get a chance?)
[09:34] <NCommander> morning bigjools
[09:39] <gmb> jml: Can I have your vote on https://code.edge.launchpad.net/~gmb/launchpad/bugwatch-next_check-bug-544238 please?
[09:39] <BjornT> gmb: sure, i'll look at it, if you explain how it will be used in more detail :)
[09:40] <gmb> BjornT: Cool. So, we want to be a bit more sensible with scheduling bug watch checks. At the moment we check each watch once every 24 hours, but if it's erroring a lot (which we now record in the BugWatchActivity table) we don't want to check it so often.
[09:41] <gmb> BjornT: So, instead of building that logic into BugTracker.getBugWatchesNeedingUpdate(), which would be complex at best, we think we should have a garbo-hourly process that looks at all the bug watches checked in the last hour, checks to see how often they've errored and then schedules their next_check accordingly.
[09:41] <gmb> So BugTracker.getBugWatchesNeedingUpdate() will look at the next_check field instead of lastchecked to determine whether a watch needs checking.
[09:42] <gmb> The advantage is that we don't have to further complicate checkwatches to get the back-off for erroring watches.
[10:00] <BjornT> gmb: ok, that makes sense. i'm wondering if we could find a better name? if you set next_check to some time, it doesn't mean that it will be checked exactly at that time, will it?
[10:01] <gmb> BjornT: Hmm, true. How about check_after?
[10:02] <stub> check_due ?
[10:02]  * stub gets out his bike shed schematics
[10:03] <gmb> It should be blue.
[10:05] <BjornT> gmb: maybe next_check is good, as long as there is a comment explaining the semantics. can't think of a better name, and i think next_check is better than the other suggestions.
[10:05] <gmb> BjornT: Right; I'll add the comment (should've added it anyway).
[10:16] <NCommander> BjornT: you around?
[10:21] <BjornT> NCommander: kind of. i'm in desperate need of lunch, though :)
[10:22] <NCommander> BjornT: ah, no rush :-)
[10:32] <NCommander> BjornT: if after lunch you'd be willing to look at my raw_source_changelog_librarian branch, I'd be most grateful
[10:32] <NCommander> stub: thanks for the review
[10:36] <BjornT> NCommander: sure. i have a bit of a backlog returning from vacation, but i'll try to get at it today
[10:36] <NCommander> BjornT: thanks
[10:36] <NCommander> BjornT: (its a new merge proposal versus the old one)
[10:44] <noodles775> intellectronica: you don't think it's worth closing the choice edit before bringing up the confirmation?
[10:45] <noodles775> (for https://code.edge.launchpad.net/~intellectronica/launchpad/bug-531963/+merge/21628)
[10:45] <intellectronica> noodles775: i do. thanks for your patch, that's exactly what i was missing :)
[10:46] <intellectronica> i didn't consider using css to get rid of it
[10:46] <noodles775> intellectronica: ah, I just didn't see it in the MP diff. Great.
[10:46] <jml> gmb, I'm confused
[10:47] <jml> gmb, do you want me to do a code review or a db review?
[10:47] <gmb> jml: Sorry, DB; typo when I requested the review.
[10:48] <jml> gmb, ta
[10:58] <NCommander> anyone around to do a code review?
[11:48] <bac> hi NCommander
[11:48] <NCommander> morning bac
[11:48] <NCommander> bac: https://code.edge.launchpad.net/~mcasadevall/launchpad/raw_source_changelog_librarian/+merge/21909
[11:48] <bac> NCommander: morning.  i'll take a look.
[11:48] <NCommander> bac: thanks:-)
[12:01] <bac> NCommander: i see that branch has already been reviewed by michael.  generally, for consistency, we have the original reviewer look at changes.
[12:04] <NCommander> bac: his comment he made earlier tonight was to have someone else look at it, else he'd look at it tonight
[12:04] <NCommander> bac: 01:57:31 < mwhudson> NCommander: sorry have to go now, i'll review it first thing tomorrow if noone else gets to it
[12:04] <bac> NCommander: ah, ok
[12:05] <NCommander> bac: sorry for the confusion
[12:05] <bac> NCommander: and is the branch really 2600 lines?
[12:05] <NCommander> bac: no, its about ~100-200, but I regenerated sample data
[12:05] <bac> ah, ok.  thanks for that
[12:05]  * bac looks
[12:05]  * NCommander hands bac the brain-bleach
[12:58] <bac> hi NCommander -- i've updated your MP.  just a few fixes and then it'll be ready to land.  thanks a ton for the branch!
[12:59] <NCommander> bac: I still need a BjornT +1 before it can land :-/
[12:59] <bac> NCommander: right
[13:00] <NCommander> bac: ugh, I missed  afew changes. I shouldn't resubmit on lack of sleep >.<;
[13:00] <bac> NCommander: np
[13:01] <bac> jpds: i don't see your MP on https://code.edge.launchpad.net/launchpad/+activereviews
[13:05] <jpds> bac: I think bigjools is dealing with it.
[13:05] <bigjools> yes
[13:09] <bac> jpds: ok, great.  please remove your name from the topic if you line up a reviewer.
[13:13] <NCommander> bac: I got everything fixed except one lint error that wasn't in code I modified. Its too long by one character, but I don't see a good way to break this up to multiple lines
[13:13] <NCommander>             DistroSeries.id == POTemplate.distroseriesID).config(distinct=True)
[13:14] <NCommander> bac: suggestions welcome
[13:14] <bac> NCommander: what file?
[13:15] <NCommander> bac: lib/lp/registry/model/distroseries.py
[13:15] <NCommander> bac: sorry :-/
[13:18] <bac> NCommander: you could drop the .config() and put it on the next line as a separate statement
[13:18] <bac> kind of cheesy.
[13:20] <NCommander> bac: er, I can?
[13:20]  * NCommander isn't sure he's reading this right them, or why there is a == on that line
[13:22] <NCommander> bac: oh, I can. bugh, brain without coffee isn't good
[13:34] <adiroiban> henninge: hi, do you have time to look at https://code.edge.launchpad.net/~adiroiban/launchpad/bug-201749/+merge/21250 ?
[13:35] <henninge> adiroiban: maybe later, I am at a sprint atm.
[13:35] <adiroiban> henninge: ok. no problem!
[13:39] <NCommander> bac: so, I fixed everything, except utilities/paste doesn't work for me (it wants a python module which I'mlooking for now)
[13:40] <bac> NCommander: you can do it by hand.  was just a suggestion
[13:40] <NCommander> bac: ah, ok
[13:40] <bac> NCommander: but i'd be interested to hear why it doesn't work.  you do have launchpad-dev-dependencies installed?
[13:41] <NCommander> bac: I do, python-clientcookie wasn't installed
[14:01] <NCommander> bac: changes made, incremental diff posted. I think I addressed all of mwh and your concerns
[14:03] <NCommander> [topic] on call: bac || reviewing:  -|| queue: [NCommander] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:03] <NCommander> er
[14:06] <bigjools> rockstar: hi, fancy a UI pre-imp check?
[14:17] <bac> NCommander: approved with one tiny change.  thanks
[14:17] <NCommander> bac: eeek, oops. Sorry. I ran that test suite, nothing blew up :-/
[14:23] <NCommander> BjornT: how goes your backlog?
[14:35] <BjornT> NCommander: slowly!
[14:35] <NCommander> BjornT: ow :-/
[14:37] <danilos> bac, hi, do you have time to take a look at https://code.edge.launchpad.net/~danilo/launchpad/bug-545070/+merge/21938? (very simple terminology change)
[14:37] <bac> danilos: yep
[14:38] <danilos> bac, thanks
[14:38] <NCommander> BjornT: if you don't have time to look at my branch today, no rush. Its a pretty simple change (one row addition, forgein key on LFA), and its not a huge priority
[14:39] <bac> danilos: r=bac
[14:39] <danilos> bac, thanks
[14:45] <BjornT> NCommander: oh, why didn't you say it was a simple change? :) approved
[14:47] <NCommander> BjornT: awesome. Now how do I get it landed? :-)
[14:48] <rockstar> bigjools, sure!
[14:48] <bigjools> rockstar: great! https://code.edge.launchpad.net/~julian-edwards/launchpad/ppa-deletion-ui/+merge/21925
[14:48] <BjornT> NCommander: if you ask bac, he might be kind enough to help you :)
[14:49] <bigjools> rockstar: there's no tests yet, just the basic implementation
[14:49] <bigjools> but feel free to have a play
[14:49] <NCommander> BjornT: should the status be set to Needs merge versus Needs Review?
[14:50] <rockstar> bigjools, if it's UI, I don't care about tests.  :)
[14:50] <bigjools> rockstar: attaboy
[14:51] <bac> NCommander: are all of your changes done and pushed?
[14:51] <NCommander> bac: yes.
[14:52] <bac> NCommander: ok, i'll send it through ec2 and land it for you
[14:52] <NCommander> bac: w00t
[15:17] <rockstar> bigjools, so, I don't feel like I know enough about Soyuz to really grok what I'm trying to review here (and there's no movie/screen shot).  How can I get to the view?
[15:19] <bigjools> rockstar: go to http://launchpad.dev/~cprov/+archive/ppa
[15:19] <bigjools> sorry I assume too much :)
[15:19] <rockstar> bigjools, :)  It's okay.
[15:20] <rockstar> bigjools, so, once a PPA is requested for deletion, can it be uploaded to?
[15:21] <rockstar> Also, "Delete PPA" shouldn't show if the deletion request has already been made.
[15:21] <bigjools> OTP, will type when I can :)
[15:22] <rockstar> I also wonder if a red notification for "Deletion in progress" is probably better, since it's more likely to grab your attention.
[15:22] <rockstar> Although the red often means "It's broken. It's broken! It's BROKEN!!!!"
[15:45] <EdwinGrubbs> bac: can you review https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-538469-invalid-upstream-link/+merge/21947
[15:47] <bac> EdwinGrubbs: yes
[16:05] <bigjools> rockstar: ok sorry let me read backscroll
[16:34] <bigjools> rockstar: gnargh too many distractions
[16:34] <bigjools> rockstar: to answer your questions, no you can't upload to it, the browser code disables it for that reason
[16:37] <bac> EdwinGrubbs: nice.  r=bac
[16:38] <EdwinGrubbs> thanks
[16:38] <rockstar> bigjools, okay.  Did you see my other suggestions?
[16:38] <bigjools> rockstar: making it red?
[16:38] <bigjools> yeah
[16:38] <bigjools> how do I do that?
[16:44] <NCommander> bac: so, stupid question, is my branch on EC2, buildbot, or? (I'm kinda confused on how trees actually land ... or how PQM works)
[16:57] <bigjools> rockstar: what's the best way of making that notification box red?
[17:06] <rockstar> bigjools, I think that there's a similar method to the info notification method that does red boxes.
[17:07] <bigjools> perfect
[17:16] <rockstar> bigjools, also, the "Delete PPA" link shouldn't show if the deletion has already been requested.
[17:16] <bigjools> rockstar: I thought about that, it was easier to leave it and make the page template different!
[17:17] <bigjools> generally I really hate links appearing and disappearing with no explanation :/
[17:17] <bigjools> but I can change it
[17:17] <bigjools> one day I hope we'll present disabled links
[17:18] <rockstar> bigjools, grey it out then?
[17:18] <bigjools> rockstar: does Link do that?
[17:19] <rockstar> bigjools, I don't think so, but it should...  :)
[17:19] <bigjools> heh
[17:19] <rockstar> bigjools, basically, you shouldn't be able to click it if it's no longer deletable (because it's already deleted)
[17:20] <bigjools> I didn't think about it too hard because it won't be around longer than 5 minutes
[17:20] <rockstar> http://validator.w3.org/feed/check.cgi?url=http%3A%2F%2Fplanet.nclug.org%2Frss20.xml
[17:20] <bigjools> in any case we're changing tack slightly, and only removing the repository, the PPA will be disabled and hidden
[17:21]  * rockstar slaps his middle click
[17:22] <rockstar> bigjools, yeah, I figured that, but to the user, it's "deleted"
[17:22] <bigjools> rockstar: yeah
[17:22] <bigjools> rockstar: so I'll make the whole thing disappear and redirect  to the user's profile page, what do you think?
[17:23] <rockstar> bigjools, yeah, that's probably a good idea.
[17:23] <rockstar> And then the info notification is something like "The PPA deletion has been requested."
[17:24] <bigjools> right
[17:24] <bigjools> in red?
[17:24] <bigjools> :)
[17:25] <rockstar> bigjools, no, at that point, blue is fine.
[17:33] <bigjools> rockstar: cool, thanks for the pre-imp
[17:33] <rockstar> bigjools, no prob.
[17:34] <bigjools> rockstar: I'll ping you for a review when it's done, apparently you need the experience :)
[17:34] <rockstar> bigjools, yes, I'm only a lowly padawan.
[17:35] <rockstar> One day I'll be a great Jedi warrior.
[17:35] <rockstar> Either that, or become Darth Vader.
[17:35] <rockstar> Depends on whether or not I fall in this river of lava.
[17:35] <bigjools> These aren't the notifications you're looking for
[17:35]  * bigjools waves hand
[17:36] <bigjools> my font made me read that as river of Java
[18:10] <bac> NCommander: can i get you to set the commit message on https://code.edge.launchpad.net/~mcasadevall/launchpad/raw_source_changelog_librarian/+merge/21909
[18:33] <NCommander> bac: done
[18:33] <NCommander> bac: did ec2 pass?
[18:33] <bac> NCommander: haven't sent it off yet.  there is a bug in our ec2 script.  will do so now that you've set the commit msg
[18:36] <leonardr> bac: very small branch https://code.edge.launchpad.net/~leonardr/launchpad/add-mutators/+merge/21965
[18:40] <bac> leonardr: done
[18:40] <leonardr> bac, great
[18:40] <bac> NCommander: i've sent your branch off to ec2 with 'ec2 land'.  if the tests pass it will go directly to pqm.  either way you'll get email
[18:41] <bac> well, theoretically
[18:44] <bac> abentley: thanks for your email yesterday about the pitiful state of +activereviews
[18:46] <abentley> bac, :-)
[19:01] <mwhudson> bac: are you running NCommander's branch through ec2 ?
[19:01]  * mwhudson reads backlog
[19:01] <mwhudson> bac: nm
[19:01] <bac> mwhudson: yeah, i just started it
[19:02] <bac> mwhudson: first attempt failed
[21:52] <bac> NCommander: i sent your branch off to ec2 twice and if failed both times.  it didn't send email, though, so i'm not sure what the problem is
[21:52] <bac> NCommander: i'm running your tests locally
[21:52] <NCommander> bac: ugh, now what? :-/
[21:52] <bac> see if i can get output locally
[21:53]  * NCommander feels like he really broke LP :-/
[21:56] <mwhudson> can someone stamp https://code.edge.launchpad.net/~mwhudson/launchpad/disable-test_import_bzrsvn/+merge/21984 pls
[22:10] <NCommander> bac: I'm probably going to be poof soonish, and then I'm travellig, so this branch might end up sitting until next week if theres a legit test failure
[22:11] <bac> NCommander: i'll let you know what happens
[22:11] <NCommander> bac: thanks.