/srv/irclogs.ubuntu.com/2009/10/06/#launchpad-reviews.txt

=== abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
=== jamalta-afk is now known as jamalta
=== Ursinha-afk is now known as Ursinha
adeuringgmb: could you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-submission-consistency-check-udev/+merge/12910 ?09:59
gmbadeuring: Sure.10:03
adeuringgmb: thanks!10:03
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com
gmbadeuring: In your tests, you need to tidy up the dicts a bit. For single item dicts you can get away with the  "{'id': 1}," style. Otherwise, opening braces should be on the end of the preceding line, not on a line of their own.10:16
adeuringgmb: right...10:16
gmbadeuring: Take a look at the dicts in the devices list on line 233 of the diff, for example.10:17
gmbadeuring: Otherwise, this is r=me.10:26
adeuringgmb: thanks!10:26
=== jtv is now known as jtv-sprint
bigjoolsgmb: got room for another review?11:08
gmbbigjools: Sure11:08
bigjoolsgar11:09
bigjoolsclicking on the wrong person in the chooser is seriously annoying, you don't get a chance to separately OK the review request :/11:09
=== bigjools changed the topic of #launchpad-reviews to: on call: gmb || reviewing: adeuring || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: bigjools || queue: [] || This channel is logged: http://irclogs.ubuntu.com
gmbSampledata change. Nooooooooooooo.11:15
stubbigjools: Feel like adding the patch-.sql file and pushing?11:19
bigjoolsstub: ohhhhhhhhhhhh fuck11:19
bigjoolsgmb: lint requires it11:19
bigjoolsschema change11:19
gmbbigjools: Yeah, I know. Wish we could exclude its largeness from the diff though.11:20
stubbigjools: Is there any foreseeable use of storing the new data in a more structured format?11:20
stubbigjools: I'm assuming it is just used raw as is by the soyuz code.11:21
gmbbigjools: Code looks food, fwiw. r=me.11:21
bigjoolsstub: no, I see this as a temporary change until OEM has all their archives in LP11:21
bigjoolsgmb: grassyass11:21
gmbbigjools: Day nardar.11:21
bigjools:)11:21
gmbWest country accented spanish. That sounds like it should be a sitcom trope.11:22
bigjoolsstub: it gets passed to the builders as-is, whence it is placed in the sources.list in the chroot11:22
stubcool. Assuming the patch is just 'ALTER TABLE foo ADD COLUMN bar text;' it all looks fine at my end.11:23
bigjoolsyep that's it11:23
bigjoolsI pushed the new rev up BTW11:23
stubbigjools: Is Mark aware of this yet? (Not that I see him having any issues with this one)11:25
* gmb really needs to remember to write an X-Chat plugin that manages the review queue. Or a bot. Or something else.11:25
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
bigjoolsstub: I think jml is doing that job now11:25
stubcool11:25
bigjoolswhich is why he's on the MP ;)11:25
allenapjml: Do you have a few minutes to look at https://code.edge.launchpad.net/~allenap/launchpad/packages-for-autoload-bug-443061/+merge/12867 -- it's a patch for the autoland problem we talked about yesterday.11:34
jmlallenap, sure.11:34
al-maisanjml: I was wondering whether you had a chance to look at lp:~al-maisan/launchpad/dd-bug-no-112:17
jmlal-maisan, no, not yet. I plan to today12:17
jml(just going through a backlog of email caused by sprinting & moving to London)12:17
jmlal-maisan, I did skim it though. I was wondering if you looked at lp:~jml/launchpad/upload-permission-joy at all?12:18
al-maisanjml: no problem12:18
al-maisanjml: you mean the refactoring you did?12:18
jmlal-maisan, that's right.12:19
jmlal-maisan, well, the second work-in-progress refactoring branch that I was doing before I changed the branch permission stuff :)12:19
al-maisanjml: the function added (person_may_edit_branch()) makes use of verify_upload()12:20
al-maisanthe function that resulted from your refactoring12:20
jmlal-maisan, yes, but there's a second & different refactoring branch that's in progress.12:21
al-maisanjml: oh12:21
al-maisanwould that be 'lp:~jml/launchpad/upload-permission-joy' ?12:21
jmlal-maisan, yes12:22
al-maisanjml: sorry, I merged lp:~jml/launchpad/upload-permission-joy into LP devel, that was my starting point12:22
al-maisanor .. wait12:23
al-maisandid I get the branch wrong .. just a sec..12:23
jmlal-maisan, I think you got the branch wrong. Your first commit says Imported jml's 'official-write-permissions' branch, and I don't see my work from u-p-j in the diff.12:23
al-maisanjml: hmm .. I see.12:23
al-maisanjml: I'm on CHR duty today but will take a look at lp:~jml/launchpad/upload-permission-joy tomorrow ..12:24
al-maisanmaybe we can have a chat then12:24
jmlal-maisan, cool. I look forward to it :)12:25
al-maisan:)12:25
=== noodles775 changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com
noodles775Hi gmb, got time for one? https://code.launchpad.net/~michael.nelson/launchpad/436182-newlines-in-sources-workaround-pt2/+merge/1291812:31
gmbnoodles775: Sure.12:33
noodles775Thanks12:34
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com
gmbnoodles775: Nice branch. r=me12:43
* gmb lunches12:43
noodles775Thanks gmb.12:43
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com
=== matsubara-afk is now known as matsubara
=== kiko is now known as kiko-phone
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: lunch, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
bacgood morning13:10
=== gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
gmbMorning bac.13:36
bachi graham.  quiet review day?13:37
gmbbac: So far. Let's not tempt fate, eh?13:37
bacgmb:  have you upgraded to karmic?  are there expected test failures for the LP suite?13:45
gmbbac: I've only upgraded my netbook so far; waiting on reports back from deryck about whether the Karmic upgrade affects the development process before doing the work machines.13:46
bacgmb: i did a netbook last week too.  did my dev box yesterday.  ran the full suite last night and got a handful of errors13:47
gmbNot unexpected, I guess.13:48
bacnope13:48
=== sinzui changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com
sinzuigmb: bac: Do either of you have time for https://code.edge.launchpad.net/~sinzui/launchpad/announcement-heading/+merge/1289813:51
bacsinzui: just claimed it13:52
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, sinzui || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com
=== mrevell is now known as mrevell-lunch
bachi sinzui.  i'm looking at your changes to create_view and create_initialized_view14:31
bacit looks good but i'm surprised you didn't use them in this branch.14:31
sinzuibac: I did. I used them to test breadcrumbs. I had to create a fake request14:32
bacsinzui: gah, nm i see it in use at th e last line14:32
bacsinzui: nice change.  r=bac14:32
sinzuithank14:33
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com
=== mrevell-lunch is now known as mrevell
=== gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
allenapbac: Can you review a very small lpbuildbot branch please? https://code.edge.launchpad.net/~allenap/lpbuildbot/avoid-deadlock/+merge/1292116:01
bacallenap: yes.  i'm about to start a 45 minute call but can look at it afterwards16:02
allenapbac: Thanks.16:02
=== allenap changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com
=== mup_ is now known as mup
gmbbac, I'll take allenap's branch16:40
=== gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
allenapThanks gmb.16:40
gmballenap: So, can you explain to my addled mind why proc.wait() is safe now that you've grabbed stdout before doing it?16:42
gmbCos reading the MP, I don't get it :)16:42
allenapgmb: proc.wait() waits for the process to finish. If it tried to write more than 64k to its stdout, it would fill the buffer and be blocked on writing, waiting for it to be read, which won't happen because we're waiting for it to finish.16:43
allenaps/it to be read/the buffer to be read/16:44
gmballenap: Ah right, I think I get it now.16:45
* gmb Needs tea16:45
gmballenap: r=me, then.16:45
allenapgmb: Danke :)16:45
gmballenap: You'll need to set the branch status manually as I'm not sub'd to it.16:45
allenapgmb: Strange, ~launchpad is subscribed to the trunk. I wonder if that's a bug.16:47
allenapgmb: Ah, I can't set it either!16:47
gmbHah.16:47
gmballenap: That might be a bug then.16:48
gmballenap: While you're here, do you ahve time for a reciprocal review?16:48
gmbIt's 300 lines, but it's mostly refactoring16:48
allenapgmb: Or some new Krypton Factoresque puzzle from the code crew.16:48
allenapgmb: Yeah, sure.16:48
gmballenap:  https://code.edge.launchpad.net/~gmb/launchpad/checkwatches-all-to-reset-bug-442993/+merge/1293016:48
gmballenap: It's a fix for bug 442993, not 422993 as the MP suggests.16:49
mupBug #442993: checkwatches --all should be --reset <bugwatch> <Launchpad Bugs:Triaged> <https://launchpad.net/bugs/442993>16:49
allenapgmb: I was looking at the bug thinking "whaaa?"16:49
gmbIndeed.16:49
gmbWhat amuses me is that someone understood that bug well enough to mark it as a dupe.16:50
=== kiko-phone is now known as kiko
jmlallenap, references to the Krypton Factor were one of the many things in "A Bit of Fry and Laurie" that I didn't get.16:55
bacgmb:  thanks for taking allenap's review.  i'm off the call now16:56
allenapjml: If those two chose to be cryptic, who am I to let you in? I mean, Stephen Fry has recently been described as the "King of Norfolk" no less.16:57
jmlallenap, well, it's more that the Krypton Factor never existed in .au16:58
jmlallenap, so jokes about it being unwatchable lack punch.16:58
leonardrbac, i'll have a fun review for you in a few minutes17:01
bacleonardr: ok17:02
leonardrbac: here you go: https://code.edge.launchpad.net/~leonardr/lazr.restful/oauth/+merge/1293117:10
=== bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com
=== beuno is now known as beuno-lunch
=== matsubara is now known as matsubara-lunch
jmlhttps://code.edge.launchpad.net/~jml/launchpad/browse-source-code/+merge/12933 when you're ready :)17:27
jmlbeuno-lunch, ^^^17:27
allenapgmb: r=me17:30
jmlbac, can you please review that branch?17:33
bacjml: sure.  after leonard's and lunch17:33
jmlbac, thanks.17:33
bacor maybe lunch then leonard's17:34
gmballenap: Awesome, thanks.17:37
gmballenap: Can you mark the mp approved please for the sake of ec2 land?17:40
gmbNot that I can't do it myself, but that still seems funky to me.17:40
allenapgmb: Sure, sorry, I forgot.17:40
gmballenap: I honestly think that if there's only one review the reviewer should be presented with that option when they approve the review. But maybe that's just me.17:41
* gmb will file a bug.17:41
allenapgmb: Yeah, it's a two-step process that I'm often doing. I think the option to default to "no change", but it would be useful if it were there.17:43
allenaps/to default/should default/17:43
gmballenap: Better still, it would happen automagically. After all, if there are n reviewers (where n > 1), who gets to decide that the whole thing is ready to land? The last reviewer? What if they're just reviewing the legal aspects of the change... etc.17:44
jmlgmb, ec2 land also has a "--force" option to set the overall mp status.17:44
jmlwell, not set, but ignore.17:44
jmlbut it needs actual votes to populate the thingummy.17:44
gmbjml: If anyone ever provides a --force option for a thing, I decline to use it, in case it will do something Bad. Even if I know it won't.17:45
allenapgmb: How about a --go-on option?17:45
gmbBut maybe that's a hangover from my old job where we had a script that needed --force, --really-force and --no-seriously in order to actually do anything useful.17:46
gmballenap: --pretty-please17:46
jmlgmb, fair call.17:46
jmlgmb, that's kind of the spirit that I added it in (i.e. don't actually use it)17:46
gmbGlad to hear it :)17:46
jmlbut yes, we need a much better way of handling the overall mp status17:46
=== jamalta is now known as jamalta-lunch
gmbjml: Incidentally, `ec2 land` is made entirely out of win. Nice job :)17:51
jmlgmb, thanks.17:52
=== jamalta-lunch is now known as jamalta
=== beuno-lunch is now known as beuno
bacleonardr: nice branch, thanks.18:09
=== bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: jml || queue: [] || This channel is logged: http://irclogs.ubuntu.com
bacjml: done.  thanks18:13
jmlbac, thank you.18:13
jmlyou wouldn't happen to know the answer to my question in #launchpad-dev, perchance18:14
leonardrbac: ok, i hope to have a corresponding lazr.restfulclient branch for you later today18:14
bacleonardr: look forward to it18:15
bacjml: for ec2 land, what does the commit message need to look like?  what parts does the script fill in for you?  will you update the message on your MP i just reviewed?18:19
jmlbac, Just the text. ec2 fills in the square bracket bits. no, I'll just use --commit-text.18:20
jmlbac, e.g. ./utilities/ec2 land https://code.edge.launchpad.net/~jml/launchpad/browse-source-code/+merge/12933 --commit-text="Change the link to browse source code to point at actual source code, not a revision log."18:20
bacjml: nice.18:21
jmlthere's a --dry-run option to let you muck around.18:21
jmloutput of that command + dryrun is: ['./utilities/ec2', 'test', '--headless', u'--email=jml@mumak.net', '-b', 'launchpad=devel', '-s', u'[r=bac][ui=beuno][bug=369273] Change the link to browse source code to point at actual source code, not a revision log.', 'bzr+ssh://bazaar.launchpad.net/~jml/launchpad/browse-source-code']18:21
=== matsubara-lunch is now known as matsubara
bacping rockstar18:59
gary_posterallenap: I have a fix for the problem that https://code.edge.launchpad.net/~allenap/launchpad/packages-for-autoload-bug-443061/+merge/12867 addresses19:24
gary_posterAFAIK19:24
gary_posterSo that you would no longer need to warn people about that problem19:24
gary_postermaxb: fwiw, ^^^19:25
maxbwhoa, your commit message has broken the layout of the BMP page ! :-)19:26
gary_posterallenap: I'll add a comment to the MP.  If this diff has landed, that's fine.  It won't cause any harm, and we can revert it later19:26
rockstarbac, hi19:29
bacrockstar: hi19:29
rockstarbac, what's up?19:30
leonardrbac: here's the lazr.restfulclient branch: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/oauth/+merge/1294019:30
bacrockstar: i was trying to use branch.getMergeProposals() from launchpadlib and am not ever getting any results.  i wonder if you've used it.19:30
rockstarbac, tarmac uses it.19:30
=== bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com
bacleonardr: will start now19:33
leonardrgreat19:33
rockstarbac, are you sure you have the right branch?19:35
bacIn [54]: br19:35
bacOut[54]: <branch at https://api.edge.launchpad.net/beta/~thumper/launchpad/code-review-history>19:35
rockstarbac, lemme look.19:37
bacthx19:37
bacnice leonardr19:40
bacrockstar: i see what is happening now20:53
rockstarbac, yes?20:53
bacbranch.getMergeProposals returns MPs proposed to merge INTO the branch not ones for where the branch is the source20:54
rockstarbac, yeah, that's what my hypothesis was, but I couldn't confirm it yet, as I was on the phone.20:55
bacrockstar: the good news is there is branch.landing_targets20:58
rockstarbac, yeah, that's what I was going to suggest, and that getMergeProposals should return all involved proposals.20:58
rockstarWe seem to have some redundancy there.  Could you file a bug on that?20:59
bacrockstar: is there redundancy or just poor documentation?20:59
rockstarbac, redundancy, as we also have branch.landing_candidates20:59
bacrockstar: oh, i hadn't looked at that one yet21:00
bacrockstar:  sure, i'll open a bug21:00
rockstarbac, thanks.21:00
bacrockstar: though landing_candidates only returns those in a final state where getMP allows you to specify the status.  just saying there is some varied functionality21:01
bacrockstar:  bug 44485421:09
mupBug #444854: API for getting a branch's merge proposals is confusing <Launchpad Bazaar Integration:New> <https://launchpad.net/bugs/444854>21:09
rockstarbac, yeah, I think that bug reflects the real problem: confusion.21:11
rockstarbac, got time for at least one little branch, maybe two?21:12
bacrockstar:  at least one21:13
bacrockstar: branches for me?21:24
rockstarbac, one coming up.  Sorry, learned another difference between lightweight and heavyweight checkouts.21:25
bacrockstar: np21:34
=== ursula is now known as Ursinha
rockstarbac, https://code.edge.launchpad.net/~rockstar/launchpad/branch-upgrade-job/+merge/1294521:36
mwhudsonbac: you there still?22:35
bachi  mwhudson22:36
=== bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
mwhudsonbac: i have a fix for a very simple bug22:36
mwhudsonbac: but the test is offensive22:36
bacgreat22:36
mwhudsonbac: here's the diff http://pastebin.ubuntu.com/287338/22:36
mwhudson(this is the bug https://bugs.edge.launchpad.net/launchpad-code/+bug/444387)22:37
mupBug #444387: productseries-codesummary.pt typo gives all series branches 31 recent revisions <post-ui-3-cleanup> <trivial> <Launchpad Bazaar Integration:Triaged> <https://launchpad.net/bugs/444387>22:37
mwhudsonbac: any ideas for how to make the test less horrid?22:37
* bac looks22:37
bacmwhudson: sorry, i can't think of how to simplify that test22:42
mwhudsonbac: oh well, i guess i'll just try to hide it with some bland prose22:43
bacmwhudson: the narrative at line 32 of your diff is a bit perplexing.  Could you expand that a bit?22:43
mwhudsonbac: heh, yes, that bit isn't finished yet...22:43
bacok22:43
bacmwhudson: based on what you have you can rs=bac if you can't find a reviewer later.22:44
mwhudsonbac: thanks22:44
mwhudsonbac: how long are you around for?22:44
bacnot long at all22:44
baci may be back later, though22:44
bacbut i'm checking out as OCR now22:44
=== bac changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
mwhudsonok22:46
barrybac: ping22:55
=== matsubara is now known as matsubara-afk
bacbarry: this change looks great.  r=bac  http://pastebin.ubuntu.com/287348/22:57
barrybac: thanks!22:58
mwhudsonbac: http://pastebin.ubuntu.com/287352/ diff now looks like this, have time to review it still?23:01
mwhudsonrockstar: hi, can you do a simple review for me?23:07
mwhudsonrockstar: https://code.edge.launchpad.net/~mwhudson/launchpad/bug-444387/+merge/1294923:11
=== jamalta is now known as jamalta-afk
rockstarmwhudson, looking.23:19
mwhudsonrockstar: thanks23:19
rockstarmwhudson, r=me.  Thanks for getting us some more test coverage.23:22
mwhudsonrockstar: thanks23:23
mwhudsonrockstar: can i bang another simple one past you?23:43
mwhudsonrockstar: https://code.edge.launchpad.net/~mwhudson/launchpad/rocketfuel-setup-ssh/+merge/1295023:46
rockstarmwhudson, r=me23:48
mwhudsonrockstar: thanks23:53

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