=== 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 | ||
adeuring | gmb: could you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-submission-consistency-check-udev/+merge/12910 ? | 09:59 |
---|---|---|
gmb | adeuring: Sure. | 10:03 |
adeuring | gmb: 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 | ||
gmb | adeuring: 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 |
adeuring | gmb: right... | 10:16 |
gmb | adeuring: Take a look at the dicts in the devices list on line 233 of the diff, for example. | 10:17 |
gmb | adeuring: Otherwise, this is r=me. | 10:26 |
adeuring | gmb: thanks! | 10:26 |
=== jtv is now known as jtv-sprint | ||
bigjools | gmb: got room for another review? | 11:08 |
gmb | bigjools: Sure | 11:08 |
bigjools | gar | 11:09 |
bigjools | clicking 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 | ||
gmb | Sampledata change. Nooooooooooooo. | 11:15 |
stub | bigjools: Feel like adding the patch-.sql file and pushing? | 11:19 |
bigjools | stub: ohhhhhhhhhhhh fuck | 11:19 |
bigjools | gmb: lint requires it | 11:19 |
bigjools | schema change | 11:19 |
gmb | bigjools: Yeah, I know. Wish we could exclude its largeness from the diff though. | 11:20 |
stub | bigjools: Is there any foreseeable use of storing the new data in a more structured format? | 11:20 |
stub | bigjools: I'm assuming it is just used raw as is by the soyuz code. | 11:21 |
gmb | bigjools: Code looks food, fwiw. r=me. | 11:21 |
bigjools | stub: no, I see this as a temporary change until OEM has all their archives in LP | 11:21 |
bigjools | gmb: grassyass | 11:21 |
gmb | bigjools: Day nardar. | 11:21 |
bigjools | :) | 11:21 |
gmb | West country accented spanish. That sounds like it should be a sitcom trope. | 11:22 |
bigjools | stub: it gets passed to the builders as-is, whence it is placed in the sources.list in the chroot | 11:22 |
stub | cool. Assuming the patch is just 'ALTER TABLE foo ADD COLUMN bar text;' it all looks fine at my end. | 11:23 |
bigjools | yep that's it | 11:23 |
bigjools | I pushed the new rev up BTW | 11:23 |
stub | bigjools: 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 | ||
bigjools | stub: I think jml is doing that job now | 11:25 |
stub | cool | 11:25 |
bigjools | which is why he's on the MP ;) | 11:25 |
allenap | jml: 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 |
jml | allenap, sure. | 11:34 |
al-maisan | jml: I was wondering whether you had a chance to look at lp:~al-maisan/launchpad/dd-bug-no-1 | 12:17 |
jml | al-maisan, no, not yet. I plan to today | 12:17 |
jml | (just going through a backlog of email caused by sprinting & moving to London) | 12:17 |
jml | al-maisan, I did skim it though. I was wondering if you looked at lp:~jml/launchpad/upload-permission-joy at all? | 12:18 |
al-maisan | jml: no problem | 12:18 |
al-maisan | jml: you mean the refactoring you did? | 12:18 |
jml | al-maisan, that's right. | 12:19 |
jml | al-maisan, well, the second work-in-progress refactoring branch that I was doing before I changed the branch permission stuff :) | 12:19 |
al-maisan | jml: the function added (person_may_edit_branch()) makes use of verify_upload() | 12:20 |
al-maisan | the function that resulted from your refactoring | 12:20 |
jml | al-maisan, yes, but there's a second & different refactoring branch that's in progress. | 12:21 |
al-maisan | jml: oh | 12:21 |
al-maisan | would that be 'lp:~jml/launchpad/upload-permission-joy' ? | 12:21 |
jml | al-maisan, yes | 12:22 |
al-maisan | jml: sorry, I merged lp:~jml/launchpad/upload-permission-joy into LP devel, that was my starting point | 12:22 |
al-maisan | or .. wait | 12:23 |
al-maisan | did I get the branch wrong .. just a sec.. | 12:23 |
jml | al-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-maisan | jml: hmm .. I see. | 12:23 |
al-maisan | jml: I'm on CHR duty today but will take a look at lp:~jml/launchpad/upload-permission-joy tomorrow .. | 12:24 |
al-maisan | maybe we can have a chat then | 12:24 |
jml | al-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 | ||
noodles775 | Hi gmb, got time for one? https://code.launchpad.net/~michael.nelson/launchpad/436182-newlines-in-sources-workaround-pt2/+merge/12918 | 12:31 |
gmb | noodles775: Sure. | 12:33 |
noodles775 | Thanks | 12:34 |
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
gmb | noodles775: Nice branch. r=me | 12:43 |
* gmb lunches | 12:43 | |
noodles775 | Thanks 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 | ||
bac | good morning | 13:10 |
=== gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
gmb | Morning bac. | 13:36 |
bac | hi graham. quiet review day? | 13:37 |
gmb | bac: So far. Let's not tempt fate, eh? | 13:37 |
bac | gmb: have you upgraded to karmic? are there expected test failures for the LP suite? | 13:45 |
gmb | bac: 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 |
bac | gmb: i did a netbook last week too. did my dev box yesterday. ran the full suite last night and got a handful of errors | 13:47 |
gmb | Not unexpected, I guess. | 13:48 |
bac | nope | 13:48 |
=== sinzui changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com | ||
sinzui | gmb: bac: Do either of you have time for https://code.edge.launchpad.net/~sinzui/launchpad/announcement-heading/+merge/12898 | 13:51 |
bac | sinzui: just claimed it | 13: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 | ||
bac | hi sinzui. i'm looking at your changes to create_view and create_initialized_view | 14:31 |
bac | it looks good but i'm surprised you didn't use them in this branch. | 14:31 |
sinzui | bac: I did. I used them to test breadcrumbs. I had to create a fake request | 14:32 |
bac | sinzui: gah, nm i see it in use at th e last line | 14:32 |
bac | sinzui: nice change. r=bac | 14:32 |
sinzui | thank | 14: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 | ||
allenap | bac: Can you review a very small lpbuildbot branch please? https://code.edge.launchpad.net/~allenap/lpbuildbot/avoid-deadlock/+merge/12921 | 16:01 |
bac | allenap: yes. i'm about to start a 45 minute call but can look at it afterwards | 16:02 |
allenap | bac: 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 | ||
gmb | bac, I'll take allenap's branch | 16:40 |
=== gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
allenap | Thanks gmb. | 16:40 |
gmb | allenap: So, can you explain to my addled mind why proc.wait() is safe now that you've grabbed stdout before doing it? | 16:42 |
gmb | Cos reading the MP, I don't get it :) | 16:42 |
allenap | gmb: 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 |
allenap | s/it to be read/the buffer to be read/ | 16:44 |
gmb | allenap: Ah right, I think I get it now. | 16:45 |
* gmb Needs tea | 16:45 | |
gmb | allenap: r=me, then. | 16:45 |
allenap | gmb: Danke :) | 16:45 |
gmb | allenap: You'll need to set the branch status manually as I'm not sub'd to it. | 16:45 |
allenap | gmb: Strange, ~launchpad is subscribed to the trunk. I wonder if that's a bug. | 16:47 |
allenap | gmb: Ah, I can't set it either! | 16:47 |
gmb | Hah. | 16:47 |
gmb | allenap: That might be a bug then. | 16:48 |
gmb | allenap: While you're here, do you ahve time for a reciprocal review? | 16:48 |
gmb | It's 300 lines, but it's mostly refactoring | 16:48 |
allenap | gmb: Or some new Krypton Factoresque puzzle from the code crew. | 16:48 |
allenap | gmb: Yeah, sure. | 16:48 |
gmb | allenap: https://code.edge.launchpad.net/~gmb/launchpad/checkwatches-all-to-reset-bug-442993/+merge/12930 | 16:48 |
gmb | allenap: It's a fix for bug 442993, not 422993 as the MP suggests. | 16:49 |
mup | Bug #442993: checkwatches --all should be --reset <bugwatch> <Launchpad Bugs:Triaged> <https://launchpad.net/bugs/442993> | 16:49 |
allenap | gmb: I was looking at the bug thinking "whaaa?" | 16:49 |
gmb | Indeed. | 16:49 |
gmb | What 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 | ||
jml | allenap, 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 |
bac | gmb: thanks for taking allenap's review. i'm off the call now | 16:56 |
allenap | jml: 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 |
jml | allenap, well, it's more that the Krypton Factor never existed in .au | 16:58 |
jml | allenap, so jokes about it being unwatchable lack punch. | 16:58 |
leonardr | bac, i'll have a fun review for you in a few minutes | 17:01 |
bac | leonardr: ok | 17:02 |
leonardr | bac: here you go: https://code.edge.launchpad.net/~leonardr/lazr.restful/oauth/+merge/12931 | 17: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 | ||
jml | https://code.edge.launchpad.net/~jml/launchpad/browse-source-code/+merge/12933 when you're ready :) | 17:27 |
jml | beuno-lunch, ^^^ | 17:27 |
allenap | gmb: r=me | 17:30 |
jml | bac, can you please review that branch? | 17:33 |
bac | jml: sure. after leonard's and lunch | 17:33 |
jml | bac, thanks. | 17:33 |
bac | or maybe lunch then leonard's | 17:34 |
gmb | allenap: Awesome, thanks. | 17:37 |
gmb | allenap: Can you mark the mp approved please for the sake of ec2 land? | 17:40 |
gmb | Not that I can't do it myself, but that still seems funky to me. | 17:40 |
allenap | gmb: Sure, sorry, I forgot. | 17:40 |
gmb | allenap: 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 | |
allenap | gmb: 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 |
allenap | s/to default/should default/ | 17:43 |
gmb | allenap: 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 |
jml | gmb, ec2 land also has a "--force" option to set the overall mp status. | 17:44 |
jml | well, not set, but ignore. | 17:44 |
jml | but it needs actual votes to populate the thingummy. | 17:44 |
gmb | jml: 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 |
allenap | gmb: How about a --go-on option? | 17:45 |
gmb | But 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 |
gmb | allenap: --pretty-please | 17:46 |
jml | gmb, fair call. | 17:46 |
jml | gmb, that's kind of the spirit that I added it in (i.e. don't actually use it) | 17:46 |
gmb | Glad to hear it :) | 17:46 |
jml | but yes, we need a much better way of handling the overall mp status | 17:46 |
=== jamalta is now known as jamalta-lunch | ||
gmb | jml: Incidentally, `ec2 land` is made entirely out of win. Nice job :) | 17:51 |
jml | gmb, thanks. | 17:52 |
=== jamalta-lunch is now known as jamalta | ||
=== beuno-lunch is now known as beuno | ||
bac | leonardr: 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 | ||
bac | jml: done. thanks | 18:13 |
jml | bac, thank you. | 18:13 |
jml | you wouldn't happen to know the answer to my question in #launchpad-dev, perchance | 18:14 |
leonardr | bac: ok, i hope to have a corresponding lazr.restfulclient branch for you later today | 18:14 |
bac | leonardr: look forward to it | 18:15 |
bac | jml: 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 |
jml | bac, Just the text. ec2 fills in the square bracket bits. no, I'll just use --commit-text. | 18:20 |
jml | bac, 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 |
bac | jml: nice. | 18:21 |
jml | there's a --dry-run option to let you muck around. | 18:21 |
jml | output 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 | ||
bac | ping rockstar | 18:59 |
gary_poster | allenap: I have a fix for the problem that https://code.edge.launchpad.net/~allenap/launchpad/packages-for-autoload-bug-443061/+merge/12867 addresses | 19:24 |
gary_poster | AFAIK | 19:24 |
gary_poster | So that you would no longer need to warn people about that problem | 19:24 |
gary_poster | maxb: fwiw, ^^^ | 19:25 |
maxb | whoa, your commit message has broken the layout of the BMP page ! :-) | 19:26 |
gary_poster | allenap: 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 later | 19:26 |
rockstar | bac, hi | 19:29 |
bac | rockstar: hi | 19:29 |
rockstar | bac, what's up? | 19:30 |
leonardr | bac: here's the lazr.restfulclient branch: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/oauth/+merge/12940 | 19:30 |
bac | rockstar: 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 |
rockstar | bac, 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 | ||
bac | leonardr: will start now | 19:33 |
leonardr | great | 19:33 |
rockstar | bac, are you sure you have the right branch? | 19:35 |
bac | In [54]: br | 19:35 |
bac | Out[54]: <branch at https://api.edge.launchpad.net/beta/~thumper/launchpad/code-review-history> | 19:35 |
rockstar | bac, lemme look. | 19:37 |
bac | thx | 19:37 |
bac | nice leonardr | 19:40 |
bac | rockstar: i see what is happening now | 20:53 |
rockstar | bac, yes? | 20:53 |
bac | branch.getMergeProposals returns MPs proposed to merge INTO the branch not ones for where the branch is the source | 20:54 |
rockstar | bac, yeah, that's what my hypothesis was, but I couldn't confirm it yet, as I was on the phone. | 20:55 |
bac | rockstar: the good news is there is branch.landing_targets | 20:58 |
rockstar | bac, yeah, that's what I was going to suggest, and that getMergeProposals should return all involved proposals. | 20:58 |
rockstar | We seem to have some redundancy there. Could you file a bug on that? | 20:59 |
bac | rockstar: is there redundancy or just poor documentation? | 20:59 |
rockstar | bac, redundancy, as we also have branch.landing_candidates | 20:59 |
bac | rockstar: oh, i hadn't looked at that one yet | 21:00 |
bac | rockstar: sure, i'll open a bug | 21:00 |
rockstar | bac, thanks. | 21:00 |
bac | rockstar: though landing_candidates only returns those in a final state where getMP allows you to specify the status. just saying there is some varied functionality | 21:01 |
bac | rockstar: bug 444854 | 21:09 |
mup | Bug #444854: API for getting a branch's merge proposals is confusing <Launchpad Bazaar Integration:New> <https://launchpad.net/bugs/444854> | 21:09 |
rockstar | bac, yeah, I think that bug reflects the real problem: confusion. | 21:11 |
rockstar | bac, got time for at least one little branch, maybe two? | 21:12 |
bac | rockstar: at least one | 21:13 |
bac | rockstar: branches for me? | 21:24 |
rockstar | bac, one coming up. Sorry, learned another difference between lightweight and heavyweight checkouts. | 21:25 |
bac | rockstar: np | 21:34 |
=== ursula is now known as Ursinha | ||
rockstar | bac, https://code.edge.launchpad.net/~rockstar/launchpad/branch-upgrade-job/+merge/12945 | 21:36 |
mwhudson | bac: you there still? | 22:35 |
bac | hi mwhudson | 22:36 |
=== bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
mwhudson | bac: i have a fix for a very simple bug | 22:36 |
mwhudson | bac: but the test is offensive | 22:36 |
bac | great | 22:36 |
mwhudson | bac: 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 |
mup | Bug #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 |
mwhudson | bac: any ideas for how to make the test less horrid? | 22:37 |
* bac looks | 22:37 | |
bac | mwhudson: sorry, i can't think of how to simplify that test | 22:42 |
mwhudson | bac: oh well, i guess i'll just try to hide it with some bland prose | 22:43 |
bac | mwhudson: the narrative at line 32 of your diff is a bit perplexing. Could you expand that a bit? | 22:43 |
mwhudson | bac: heh, yes, that bit isn't finished yet... | 22:43 |
bac | ok | 22:43 |
bac | mwhudson: based on what you have you can rs=bac if you can't find a reviewer later. | 22:44 |
mwhudson | bac: thanks | 22:44 |
mwhudson | bac: how long are you around for? | 22:44 |
bac | not long at all | 22:44 |
bac | i may be back later, though | 22:44 |
bac | but i'm checking out as OCR now | 22:44 |
=== bac changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
mwhudson | ok | 22:46 |
barry | bac: ping | 22:55 |
=== matsubara is now known as matsubara-afk | ||
bac | barry: this change looks great. r=bac http://pastebin.ubuntu.com/287348/ | 22:57 |
barry | bac: thanks! | 22:58 |
mwhudson | bac: http://pastebin.ubuntu.com/287352/ diff now looks like this, have time to review it still? | 23:01 |
mwhudson | rockstar: hi, can you do a simple review for me? | 23:07 |
mwhudson | rockstar: https://code.edge.launchpad.net/~mwhudson/launchpad/bug-444387/+merge/12949 | 23:11 |
=== jamalta is now known as jamalta-afk | ||
rockstar | mwhudson, looking. | 23:19 |
mwhudson | rockstar: thanks | 23:19 |
rockstar | mwhudson, r=me. Thanks for getting us some more test coverage. | 23:22 |
mwhudson | rockstar: thanks | 23:23 |
mwhudson | rockstar: can i bang another simple one past you? | 23:43 |
mwhudson | rockstar: https://code.edge.launchpad.net/~mwhudson/launchpad/rocketfuel-setup-ssh/+merge/12950 | 23:46 |
rockstar | mwhudson, r=me | 23:48 |
mwhudson | rockstar: thanks | 23:53 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!