| thumper | mwhudson: could you please look over the above review? | 01:46 |
|---|---|---|
| thumper | mwhudson: it's short and will please the losas | 01:47 |
| mwhudson | thumper: okay | 01:47 |
| thumper | thanks | 01:47 |
| mwhudson | thumper: the diff lacks enough context for me to tell this, but i presume there's a test that a random user cannot make a branch private? | 01:52 |
| EdwinGrubbs | mwhudson: are you the OCR today? | 01:53 |
| mwhudson | EdwinGrubbs: i guess i am | 01:53 |
| mwhudson | thumper: also should special users be allowed to make any branch public? | 01:54 |
| mwhudson | thumper: it doesn't look like they can here? | 01:54 |
| EdwinGrubbs | mwhudson: can you review a 125 line diff for me? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-297833-invite-private-team/+merge/15258 | 01:54 |
| mwhudson | EdwinGrubbs: looking | 01:55 |
| mwhudson | EdwinGrubbs: self.request.form.get('field.newmember') is pretty horrid | 01:57 |
| mwhudson | EdwinGrubbs: is there no way of getting a widgets value if it's not considered valid by the Field? | 01:58 |
| mwhudson | (i can believe there's not but it's a bit sad) | 01:58 |
| mwhudson | thumper, EdwinGrubbs: these are not a very online reviews | 02:04 |
| mwhudson | EdwinGrubbs: i don't see how to reproduce the "I also noticed that the $team/+add-my-teams would OOPS if you" part | 02:32 |
| thumper | mwhudson: yes there is | 02:40 |
| thumper | mwhudson: I did think about making any branch public and decided against it | 02:40 |
| thumper | mwhudson: because the only time they can't is if the policy says private only | 02:40 |
| mwhudson | thumper: oh right | 02:40 |
| thumper | mwhudson: I wanted the UI to not allow them to shoot themselves in the foot | 02:41 |
| thumper | mwhudson: they technically could through the api | 02:41 |
| mwhudson | yeah, that's fair enough | 02:41 |
| thumper | mwhudson: but the UI is a bit safer | 02:41 |
| mwhudson | thumper: reviewed | 02:45 |
| thumper | mwhudson: ta | 02:47 |
| * mwhudson goes to buy some cake | 02:50 | |
| mwhudson | thumper: want to review a bzr-svn branch? | 04:43 |
| EdwinGrubbs | mwhudson: sorry about disappearing after you started the review. I'll find someone else to finish it. Thanks for the feedback. | 05:09 |
| thumper | mwhudson: sure | 07:00 |
| mwhudson | thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/bzr-svn-monitor/+merge/15264 | 07:01 |
| mwhudson | thumper: my plan is to land this, and enough to stop the ui blowing up when it sees a bzr-svn import | 07:01 |
| mwhudson | then we can test on staging by getting the losas to make bzr-svn imports with sql | 07:01 |
| mwhudson | and if they work and add the ui to create them on edge | 07:01 |
| === noodles775 changed the topic of #launchpad-reviews to: on-call: noodles775 || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | noodles775: got one for you... https://bugs.edge.launchpad.net/rosetta/+bug/488218 | 10:40 |
| noodles775 | jtv: on it. | 10:40 |
| jtv | mp at https://code.edge.launchpad.net/~jtv/launchpad/bug-488218/+merge/15247 | 10:41 |
| jtv | thanks! | 10:41 |
| === noodles775 changed the topic of #launchpad-reviews to: on-call: noodles775 || reviewing: jtv || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| bac | jml: in light of the current problem with ec2 failing i've added an option to ec2 land to make it *not* go headless. would you like to review it? | 11:51 |
| jml | bac, sure, but I don't see how that helps. | 11:52 |
| jml | bac, wouldn't it be better to address the root cause of the problem? | 11:52 |
| bac | jml: i assume mwhudson is working on the root cause. staying attached did help me get my branches landed last night but i thought it might be general enough to be useful in the future. | 11:53 |
| jml | bac, oh right, an option. sorry I misread :) | 11:54 |
| bac | jml: ec2 land --attached | 11:54 |
| bac | using land and going headless are somewhat orthogonal | 11:54 |
| bac | though i agree headless should be the default | 11:54 |
| jml | bac, agreed. | 11:56 |
| jml | bac, I didn't add the option because I couldn't think of a use-case for attached mode. | 11:56 |
| jml | (fwiw, I wouldn't assume mwhudson is working on it myself) | 11:56 |
| bac | jml: i'm not sure if mwhudson is or not. this work was done last night before the cause of the failure was discovered. i assume the Build Engineer (who's that now?) will have a look. | 11:58 |
| bac | jml: i would but i'm on holiday today | 11:58 |
| bac | and about to head out | 11:59 |
| jml | bac, oh right, I forgot about the turkeys. | 11:59 |
| bac | jml: anyway, a MP was just submitted. if you want to review it please do. i'll check back later and respond/land. | 11:59 |
| jml | bac, will do. | 11:59 |
| bac | jml: thanks! | 12:00 |
| gmb | noodles775: Can I stick https://code.edge.launchpad.net/~gmb/launchpad/prince-of-ajax-dupefinding-bug-358510/+merge/15273 on your queue? | 12:48 |
| noodles775 | yup :) | 12:48 |
| gmb | Kewl. | 12:48 |
| === gmb changed the topic of #launchpad-reviews to: on-call: noodles775 || reviewing: jtv || queue [gmb] || 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 || reviewing: gmb || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | gmb: where is launchpad_form_id actually set? I assume a parent template? | 14:29 |
| gmb | noodles775: Yes. It's not used in this branch, it will be used in subsequent branches. I can land it with one of those if you'd prefer. | 14:29 |
| noodles775 | gmb: well, there's a number of things in this branch that indicated that you wouldn't be landing this? (empty errors, 'FAIL' etc.). Is there any reason to land this? Why can't it be landed with the subsequent branches? | 14:31 |
| noodles775 | or is that what you meant? | 14:31 |
| gmb | noodles775: Damn, I must've missed trimming that crap out. Sorry. I've had to merge this in from old branches bits and pieces at a time. | 14:32 |
| gmb | noodles775: And I don't want to land it all in subsequent branches because it's 400+ lines already. | 14:32 |
| gmb | noodles775: As a whole, this thing is well over 1200 lines. | 14:32 |
| gmb | So I've got to land it in bits. | 14:33 |
| noodles775 | gmb: but that shouldn't matter (using the 'Prerequisite branch' option, and/or pipelines)? | 14:33 |
| noodles775 | I landed a 3.5k branch which was 7 separate reviews... it should work? | 14:34 |
| noodles775 | But that's up to you. | 14:34 |
| noodles775 | It's just quite difficult to review without being able to see it, or test it. | 14:35 |
| gmb | noodles775: Okay. Leave the review for now; I'll have to get back to you. | 14:37 |
| gmb | Thanks anyway. | 14:37 |
| noodles775 | gmb: I'm happy to r=me if it's not going to land until it's tested in subsequent branches etc. | 14:37 |
| gmb | noodles775: In that case, please do. I'll have another branch ready by tomorrow anyway. | 14:38 |
| gmb | noodles775: Sorry, just having to juggle things here at the moment. | 14:38 |
| noodles775 | OK. | 14:38 |
| noodles775 | Np. | 14:38 |
| jtv1 | gmb: almost done with that branch... just cleaning up the test | 14:44 |
| === jtv1 is now known as jtv | ||
| === noodles775 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 | ||
| BjornT_ | noodles775: landing big branches is technically possible, but should still be avoided. big diffs makes it harder to back out revisions, and it's easier for mistakes to slip through. some of us actually look at the diff that lands :) | 15:00 |
| noodles775 | BjornT_: Yeah, I agree of course, but I still think the smaller branches need to be complete units of work (ie. that are tested) | 15:03 |
| BjornT_ | noodles775: yes, of course | 15:04 |
| noodles775 | For the large branch that I mentioned above, it was for the PPA index redesign... I landed all the backend branches in small parts where possible, but there remained a large number of dependent branches that couldn't be landed on their own. | 15:04 |
| noodles775 | Maybe I just didn't find the best sequence of small steps. | 15:04 |
| === salgado is now known as salgado-lunch | ||
| jtv | gmb: requested review from you. | 15:27 |
| gmb | jtv: Cool. I'll look at it shortly. | 15:27 |
| jtv | gmb: I'll have to eod now and give my wrists a rest. | 15:27 |
| jtv | The test is almost all the work. | 15:27 |
| gmb | Righto. | 15:28 |
| === matsubara is now known as matsubara-lunch | ||
| === beuno is now known as beuno-lunch | ||
| === salgado-lunch is now known as salgado | ||
| === jtv is now known as jtv-afk | ||
| === matsubara-lunch is now known as matsubara | ||
| === beuno-lunch is now known as beuno | ||
| === ursula is now known as Ursinha | ||
| === salgado is now known as salgado-afk | ||
| abentley | thumper: Any chance you could review https://code.edge.launchpad.net/~abentley/launchpad/fix-inline-reply/+merge/15295 ? | 20:40 |
| thumper | abentley: ok | 20:42 |
| === matsubara is now known as matsubara-afk | ||
| === mwhudson_ is now known as mwhudson | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!