| mwhudson | thumper: can you review https://code.edge.launchpad.net/~mwhudson/launchpad/reduce-concurrent-job-count/+merge/19574 pls? | 01:26 |
|---|---|---|
| * thumper looks | 01:27 | |
| thumper | mwhudson: are you happy with the value 3? | 01:30 |
| thumper | mwhudson: I'm wondering what we'll do with a new quad core machine soon | 01:30 |
| mwhudson | thumper: yeah, that did occur to me too | 01:30 |
| thumper | mwhudson: anyway, happy for this to go in. | 01:31 |
| thumper | r=me | 01:31 |
| mwhudson | thumper: maybe i should change it to be a paramter to getJobsForMachine, and have the slave pass it in | 01:31 |
| mwhudson | that's much easier to change via cp | 01:31 |
| thumper | yeah | 01:31 |
| thumper | can we have it as a command line param? | 01:32 |
| thumper | or at least have a command line override the default if supplied | 01:32 |
| thumper | that way we can keep this config variable | 01:32 |
| * thumper runs to collect Rachel and a coffee | 01:32 | |
| mwhudson | thumper: want to review incremental-code-imports-bug-512683 ? | 02:24 |
| thumper | mwhudson: sure | 02:24 |
| * mwhudson boggles | 02:24 | |
| mwhudson | i can't type new lines into this textarea | 02:24 |
| * mwhudson restarts ff, it works now | 02:26 | |
| mwhudson | stupid free software crap | 02:26 |
| mwhudson | <wink> | 02:27 |
| mwhudson | thumper: you have mail | 02:40 |
| thumper | ok | 02:55 |
| thumper | mwhudson: damn out of order emails | 02:55 |
| mwhudson | thumper: :-) | 02:56 |
| thumper | mwhudson: is 5k too much? | 02:56 |
| thumper | mwhudson: how many on the kernel before it goes slow? | 02:56 |
| mwhudson | thumper: ot | 02:56 |
| mwhudson | it's a bit of a guess, to be sure | 02:56 |
| * mwhudson goes to look at some log files | 02:56 | |
| mwhudson | it probably is too many, takes about 12 hours for the kernel? | 02:58 |
| mwhudson | 2500 took about 6 hours | 02:59 |
| thumper | 1000? | 02:59 |
| mwhudson | 1000 about 1hr 30 | 02:59 |
| thumper | lets use that | 02:59 |
| mwhudson | thumper: ok | 03:00 |
| thumper | mwhudson: I don't have an issue with an intial git import taking 20 or 30 partial successes | 03:00 |
| thumper | mwhudson: is this mainline revisions or total revisions? | 03:00 |
| mwhudson | thumper: total | 03:00 |
| mwhudson | thumper: linux is 168 k :-) | 03:00 |
| thumper | hmm... | 03:01 |
| thumper | well, worth a crack nigel | 03:01 |
| mwhudson | it'll be easy to chage with a cowboy | 03:01 |
| mwhudson | you could even do a time based limit i guess | 03:02 |
| mwhudson | though that'd require changing bzr-git again | 03:03 |
| thumper | mwhudson: in test_worker don't you want TestCaseWithFactory rather than just TestCase? | 03:04 |
| mwhudson | thumper: no, these tests don't interact with the database | 03:05 |
| thumper | mwhudson: ok | 03:05 |
| mwhudson | thumper: small changes pushed btw | 03:05 |
| mwhudson | (i forgot to commit some self-review) | 03:05 |
| === _thumper_ is now known as thumper | ||
| mwhudson | thumper: thanks for the review | 03:32 |
| thumper | mwhudson: np | 03:32 |
| thumper | it is Friday afternoon | 03:32 |
| mwhudson | thumper: do you think we should get a UI review? | 03:33 |
| thumper | short answer: no | 03:33 |
| mwhudson | ok | 03:33 |
| thumper | we may want to get someone to look at it later | 03:33 |
| thumper | but we shouldn't block on it | 03:33 |
| mwhudson | yeah ok | 03:33 |
| thumper | the actual ui change is almost unnoticable | 03:33 |
| thumper | and only visible in certain early circumstances | 03:34 |
| thumper | noticable by us, but probably not too many others | 03:34 |
| * mwhudson ec2 lands | 03:34 | |
| mwhudson | thumper: can you look at https://code.edge.launchpad.net/~mwhudson/launchpad/reduce-concurrent-job-count/+merge/19574 again? | 03:36 |
| thumper | ok | 03:36 |
| mwhudson | ta | 03:42 |
| thumper | mwhudson: email coming your way | 03:47 |
| mwhudson | heh, that's what i get for not writing tests i guess | 03:49 |
| mwhudson | *running | 03:50 |
| mwhudson | (also coding on a friday afternoon) | 03:50 |
| thumper | https://devpad.canonical.com/~tim/description1.png | 03:59 |
| thumper | https://devpad.canonical.com/~tim/description2.png | 03:59 |
| thumper | https://devpad.canonical.com/~tim/description3.png | 03:59 |
| thumper | https://devpad.canonical.com/~tim/description4.png | 03:59 |
| thumper | https://devpad.canonical.com/~tim/description5.png | 03:59 |
| thumper | mwhudson: ^^ | 03:59 |
| thumper | comments? | 03:59 |
| mwhudson | thumper: why a private server? | 04:00 |
| * mwhudson looks | 04:00 | |
| mwhudson | bah | 04:00 |
| thumper | mwhudson: because it is easy? | 04:00 |
| * mwhudson wants focus follows eyes | 04:00 | |
| mwhudson | fair enough | 04:00 |
| thumper | I could have copied them to penhey.net... | 04:00 |
| mwhudson | train your fingers to type people.canonical.com instead :-) | 04:00 |
| thumper | I have people.canonical.com? | 04:00 |
| mwhudson | yes | 04:00 |
| thumper | where? | 04:00 |
| mwhudson | tim@ | 04:01 |
| thumper | or is it devpad? | 04:01 |
| mwhudson | same as everything else | 04:01 |
| mwhudson | no | 04:01 |
| mwhudson | it used to be rookery, but it's another machine now | 04:01 |
| mwhudson | lillypilly or something | 04:01 |
| thumper | heh, first time I've logged in to there | 04:01 |
| thumper | ok all on https://people.canonical.com/~tim/description*.png too | 04:03 |
| mwhudson | thumper: do you have a screeny of the description in the lower position when both description and commit message are set? | 04:03 |
| * thumper makes one | 04:03 | |
| mwhudson | thumper: boring one for you | 04:05 |
| mwhudson | thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/bzr-2.1-gold/+merge/19678 | 04:05 |
| thumper | http://people.canonical.com/~tim/description6.png | 04:07 |
| thumper | mwhudson: 6400 lines isn't trivial :) | 04:07 |
| thumper | mwhudson: proposed against db-devel? | 04:07 |
| mwhudson | thumper: crap | 04:08 |
| mwhudson | yeah | 04:08 |
| thumper | mwhudson: I want to be able to have an editor to change that | 04:08 |
| thumper | mwhudson: to save the whole delete, recreate screwage | 04:08 |
| mwhudson | thumper: yes please | 04:09 |
| mwhudson | thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/bzr-2.1-gold/+merge/19679 then | 04:09 |
| thumper | done | 04:11 |
| mwhudson | three things in ec2 | 04:11 |
| mwhudson | might be enough for today... | 04:12 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | on call: adeuring || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | 09:01 |
| henninge | adeuring: was that ^ meant to go into the subject? ;-) | 09:07 |
| adeuring | henninge: argh, yes... | 09:07 |
| * adeuring needs more coffee | 09:07 | |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | adeuring: cool, that means you have time for my branch! ;-) | 09:08 |
| adeuring | henninge: sure, but first more coffee ;) | 09:08 |
| henninge | adeuring: whatever you need ... ;) | 09:08 |
| henninge | https://code.edge.launchpad.net/~henninge/launchpad/bug-523810-needs-information-age/+merge/19689 | 09:08 |
| === henninge changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | adeuring: thanks | 09:09 |
| adeuring | henninge: review sent. I have a few questions | 10:24 |
| henninge | adeuring: thanks, I'll look at it | 10:25 |
| henninge | adeuring: about the placement of the constants in the inteface module. | 10:29 |
| adeuring | henninge: yes? | 10:29 |
| henninge | adeuring: I remember a rule that browser and test code should not import from model code. | 10:30 |
| adeuring | henninge: yes, you're right | 10:30 |
| henninge | adeuring: I have to admit, I have not found anybody with that same memory yet .... ;) | 10:30 |
| henninge | adeuring: oh, there you are! ;-D | 10:30 |
| henninge | adeuring: that's why I moved this into the interface module. | 10:31 |
| adeuring | ok, so let's leave it there | 10:31 |
| henninge | adeuring: also, I consider these expiration times as implementation-independent. | 10:31 |
| adeuring | ok | 10:31 |
| henninge | and that is the idea of the interface-model split, isn't? | 10:31 |
| adeuring | henninge: in general, yes. But it is a bit ober-bureaucratic in ths case, IHMO ;) | 10:32 |
| adeuring | s/ober/over/ | 10:33 |
| henninge | ;) | 10:33 |
| henninge | adeuring: but I don't see a pressing reason not to have this in the interface module | 10:34 |
| adeuring | right | 10:34 |
| === henninge changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | adeuring: replied ;) | 11:05 |
| * adeuring is looking | 11:05 | |
| adeuring | henninge: r=me. Thanks for the changes! | 11:09 |
| gmb | adeuring: Do you have time to review https://code.edge.launchpad.net/~gmb/launchpad/link-attachment-bug-524296/+merge/19694 ? | 11:09 |
| henninge | adeuring: thank you | 11:11 |
| === gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [gmb] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| adeuring | gmb: r=me, one small additional request | 11:32 |
| gmb | adeuring: Thanks. | 11:32 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews abgeändert | ||
| adeuring | gmb: welcome :) | 11:33 |
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews abgeändert | ||
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || queue [] || 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: adeuring,salgado || reviewing: - || queue [noodles] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | Hi guys, one for the queue: https://code.launchpad.net/~michael.nelson/launchpad/ppa-privatisation-test-refactor2/+merge/19696 | 12:09 |
| noodles775 | ... if you don't mind me being gone for lunch :) | 12:10 |
| adeuring | noodles775: I'll have lunch too soon, but after that i can take it. | 12:11 |
| === mrevell is now known as mrevell-lunch | ||
| bac | hi adeuring, salgado: can i add https://code.launchpad.net/~bac/launchpad/bug-487793/+merge/19700 to the queue? | 12:30 |
| === bac changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || queue [noodles,bac] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | bac, sure; I'll take it | 12:31 |
| bac | thx | 12:31 |
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,noodles || queue [bac] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -,bac || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | Thanks for the review salgado | 13:22 |
| noodles775 | Sheesh, I've no idea what the garbage in the testing factory... | 13:24 |
| adeuring | salgado: can you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/bug-201015-bug-branch-search/+merge/19699? | 13:24 |
| salgado | adeuring, sure. would you like to take bac's so that I go straight to yours? otherwise I'll do yours after I finish his | 13:25 |
| adeuring | salgado: sure, np | 13:25 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: bac, adeuring || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === mrevell-lunch is now known as mrevell | ||
| noodles775 | salgado: so the stuff that looks like garbage in factory.py is actually in trunk, it's a sample diff used by some factory methods. My editor automatically removed some trailing whitespace from it. | 13:28 |
| salgado | oh, I see. the lines starting with a white space followed by a '+' are just context to your diff | 13:29 |
| noodles775 | Yep. | 13:29 |
| salgado | cool | 13:30 |
| jtv | salgado, one more for the queue: https://code.launchpad.net/~jtv/launchpad/bug-523926/+merge/19704 | 13:30 |
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: bac, adeuring || queue [jtv] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | thanks | 13:30 |
| salgado | adeuring, did you ask for a UI review of your branch? a UI reviewer might have other ideas for where to place the new radio button | 13:45 |
| adeuring | salgado: no, not yet... | 13:45 |
| salgado | adeuring, is there a compelling use case for including only the bugs *without* a linked branch? I don't see a similar option for including only bugs without patches/CVEs | 13:49 |
| salgado | disclaimer: I haven't read the bug report yet | 13:50 |
| === sinzui changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: bac, adeuring || queue [jtv, sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | salgado: well, the wish is mentioned in the bug report ;) | 13:50 |
| adeuring | salgado: and a use case is: "look for bugs that seem to be uninished", for example | 13:50 |
| salgado | adeuring, right, but having support for that makes the radio button necessary, thus adding some complexity and making for a not-very-clear UI | 13:53 |
| adeuring | right | 13:53 |
| salgado | I'd rather address just the most common use case (searching only for bugs with linked branches), which would allow for a simpler UI. and later, if necessary, we can extend it | 13:55 |
| salgado | but if we can come up with a simple UI that accommodates both use cases, I'd be ok, I think | 13:56 |
| salgado | although we might be adding a feature that may never be used | 13:56 |
| salgado | adeuring, did you have a pre-implementation call? | 13:56 |
| adeuring | salgado: no... | 13:57 |
| adeuring | salgado: shall we also ask the ui reviewer about the "no branches" option? | 13:57 |
| salgado | adeuring, sure, but it might be a good idea to first decide whether or not it's worth allowing users to search for bugs without branches | 13:58 |
| adeuring | salgado: well, one option is to search fro bugs which are "unfinished", even if this simply means that a branch in fact exists but is not linked. | 14:00 |
| adeuring | We have the use case to better suport upstream | 14:00 |
| adeuring | so being able to provide upstream with branches for bugs seems to be useful | 14:00 |
| salgado | adeuring, can you bring that up for discussion with deryck? I think you guys are better equipped than myself to decide whether or not it's useful | 14:01 |
| adeuring | ok | 14:01 |
| salgado | I was just trying to ensure we discussed it before going any further, to avoid having to rework later or trying to simplify a UI that's may not be necessary | 14:02 |
| * deryck looks at the initial bug report | 14:03 | |
| deryck | adeuring, salgado -- I haven't looked at the UI for this, i.e. haven't branched and run this myself, but if the UI is horrible to enable the negative, I would lean toward just searching for bugs *with* branches. | 14:08 |
| adeuring | deryck: let me make a sreecnshot... | 14:09 |
| deryck | adeuring, salgado -- hearing you guys discuss this, though, I do wonder why a radio button is required. maybe a "with branch" and "without branch" check box, and just form validate that only one is selected. Or else do nothing different than the default. :-) | 14:09 |
| deryck | adeuring, ok, will wait on screenie | 14:10 |
| adeuring | deryck: an interesting idea ;) | 14:10 |
| salgado | deryck, that wouldn't be a great UI either, I think | 14:11 |
| adeuring | deryck, salgado: http://people.canonical.com/~adeuring/branchsearch.png | 14:11 |
| deryck | adeuring, salgado -- so I don't think the radio is bad. I think it should be made to look like the affects option -- a check for "show with branches" (or some better phrase) and then "with linked branch" and "without linked branch" toggle. | 14:16 |
| deryck | we shouldn't have to have it toggled on for the default. but admittedly, I'm thinking only UI here and not what's involved to code this. | 14:17 |
| salgado | ain't that how we should think when designing the UI? ;) | 14:17 |
| adeuring | deryck: nice idea! | 14:18 |
| deryck | heh | 14:18 |
| deryck | salgado, adeuring -- of course, this is just an idea. I'm not a qualified UI reviewer. :-) I would pre-imp with a ui person. | 14:18 |
| adeuring | deryck: but... I think I know what you mean, but the "any" and "all" buttons below "show bugs affecting me" relalte to the "tags" field on left ;) | 14:19 |
| salgado | yeah, it took me ages to realize that | 14:19 |
| deryck | yeah, so it's not a perfect idea. and the grouping of the options could generally be better. | 14:21 |
| salgado | adeuring, care to check with a UI reviewer what they think of these two options? | 14:22 |
| adeuring | noodles775, sinzui: ^^^^ | 14:22 |
| sinzui | adeuring: noodles775: I can do it in 30 minutes | 14:24 |
| noodles775 | OK, thanks sinzui. | 14:25 |
| === BjornT changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: bac, adeuring || queue [jtv, sinzui, bjornt] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| BjornT | adeuring, salgado: here's a small patch for you to review when you have time: https://code.edge.launchpad.net/~bjornt/launchpad/quiet-update-sourcecode/+merge/19710 | 14:36 |
| adeuring | I'll look at it | 14:37 |
| BjornT | thanks | 14:37 |
| === salgado is now known as salgado-lunch | ||
| === sinzui changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: bac, adeuring || queue [jtv, sinzui, bjornt, sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | BjornT: r=me | 15:04 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: jtv, adeuring || queue [sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| BjornT | thanks adeuring | 15:08 |
| === matsubara is now known as matsubara-lunch | ||
| === salgado-lunch is now known as salgado | ||
| === bigjools changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: jtv, adeuring || queue [sinzui, sinzui, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | rockstar: can you check if I understood you correctly? https://code.edge.launchpad.net/~jtv/launchpad/bug-523449/+merge/19622 | 16:09 |
| jtv | rockstar: and its sibling, https://code.edge.launchpad.net/~jtv/launchpad/bug-507681/+merge/19531 | 16:10 |
| sinzui | adeuring: I am sorry my review is taking so long. bugs advanced search has many ui bugs that have irked me. Instead of doing a simple review, I am looking at why this page is different from other forms and will propose a fix instead of asking you to find the fix | 16:10 |
| adeuring | sure, sounds good! | 16:10 |
| rockstar | jtv, so basically, you kept moved some work into the second branch that could have been in the first? | 16:11 |
| jtv | rockstar: yes | 16:12 |
| jtv | rockstar: I moved the "if work_is_appropriate: work()" structure into the utility. | 16:12 |
| jtv | Could've done better, I know. | 16:13 |
| rockstar | jtv, it's fine, was just a bit confusing. | 16:13 |
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: jtv, adeuring || queue [bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-lunch is now known as matsubara | ||
| salgado | bigjools, around? | 16:24 |
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: jtv, bigjools || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | adeuring, I'll get back to yours once we've heard from sinzui, ok? | 16:25 |
| adeuring | salgado: sur | 16:25 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -, bigjools || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | adeuring: salgado: I am almost done. I have discovered that advanced search is not just a handcrafted form, all the markup and css rules for legend were updated for all forms except this one. I think I can propose a fix | 16:27 |
| adeuring | sounds good | 16:28 |
| salgado | cool; that's great | 16:28 |
| bigjools | salgado: yes | 16:34 |
| salgado | bigjools, I was just wondering if it'd be possible to use a newly created distribution in that test, to have it not depending on any sample data | 16:35 |
| bigjools | salgado: that would be quite a lot of work :/ | 16:36 |
| salgado | I imagined | 16:37 |
| bigjools | the other tests in the file all do the same thing | 16:37 |
| salgado | bigjools, do we need to have the suffixes hard-coded there? can't we take them from a constant somewhere? | 16:38 |
| bigjools | salgado: that's the only place they will appear - is there a benefit to doing that? | 16:38 |
| salgado | in that case, no | 16:39 |
| salgado | approved. :) | 16:39 |
| bigjools | one of these days we'll get rid of the sampledata crack for Soyuz | 16:39 |
| === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: -, - || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| bigjools | salgado: cheers | 16:40 |
| bigjools | salgado: I'd love to replace this particular sample data with a function that will set it up instead | 16:40 |
| bigjools | check out lucilleconfig if you want to see why that's tough :) | 16:41 |
| salgado | I believe you it's tough, and that's why I'm not convinced we'll ever manage to get rid of sampledata for soyuz/registry/bugs tests | 16:42 |
| bigjools | well tough doesn't mean impossible :) | 16:43 |
| bigjools | it will just take a very long time :/ | 16:43 |
| jtv | rockstar: any chance of finishing the review today? | 17:13 |
| rockstar | jtv, which review? | 17:13 |
| jtv | rockstar: oh, I since you commented, I thought you were reviewing my branch. | 17:13 |
| jtv | well, and since you promised to look at the changes. :-) | 17:13 |
| rockstar | jtv, haven't I approved everything outstanding? | 17:14 |
| * rockstar looks | 17:14 | |
| jtv | rockstar: ah, so you did! Thanks. Whole lotta landing coming up... | 17:16 |
| rockstar | abentley, https://code.edge.launchpad.net/~rockstar/launchpad/bug-517266/+merge/19730 | 17:51 |
| === deryck is now known as deryck[lunch] | ||
| === danilos is now known as daniloff | ||
| * maxb would like to enqueue https://code.edge.launchpad.net/~maxb/launchpad/bug-497731/+merge/19732 | 18:47 | |
| salgado | maxb, I'll take it | 18:52 |
| maxb | thanks | 18:53 |
| === deryck[lunch] is now known as deryck | ||
| salgado | maxb, is get_bzr_plugins_path() still used anywhere? | 19:03 |
| maxb | Nowhere outside that file. I was wondering whether to remove it from __all__ | 19:03 |
| salgado | yeah, that's why I asked. maybe we should just change it instead of having a separate one? | 19:05 |
| salgado | after all, the only behaviour we want is the one of get_BZR_PLUGIN_PATH_for_subprocess(), right? | 19:06 |
| maxb | It wouldn't even suck to just inline get_bzr_plugins_path at its two call-sites in lp/codehosting/__init__.py | 19:07 |
| maxb | I agree I can't imagine why any code other than the load_plugins([...]) call itself should want the directory alone | 19:08 |
| salgado | I'd be against inlining, but I'm all for keeping just one version of it -- the one that does the right thing. :) | 19:09 |
| maxb | So your suggestion is to just drop it from __all__ ? | 19:11 |
| salgado | no, just remove get_BZR_PLUGIN_PATH_for_subprocess() and change get_bzr_plugins_path() to append a "-site" to its return value | 19:12 |
| maxb | That's not acceptable because the bare directory is required for the load_plugins([...]) call ~10 lines below | 19:16 |
| salgado | oh, I missed that | 19:17 |
| salgado | why is it required there? | 19:18 |
| maxb | The way it works is that if you're calling that bzrlib call directly, bzrlib uses *exactly* the directories you pass it. If you're setting the environment variable, bzrlib augments it with the standard directories unless you use the magic tokens to tell it not to | 19:19 |
| salgado | I see | 19:20 |
| sinzui | bac: I approved the UI, but you need to fix the links first. | 19:20 |
| bac | sinzui: i'm just reading it now | 19:21 |
| sinzui | EdwinGrubbs: I just used the link to upstream multi-step form. It is very nice. It feels faster than the series picker. I think that has a lot to do with the design shown only the information you need to know | 19:22 |
| salgado | maxb, then we can rename the existing one to _get_bzr_plugins_path() and remove it from __all__ | 19:22 |
| maxb | Sounds good | 19:22 |
| salgado | maxb, also, it'd be nice to state in the new docstring why we use the "-site" magic token | 19:23 |
| EdwinGrubbs | cool | 19:23 |
| salgado | 65 args, env_changes={'BZR_SSH': 'paramiko', | 19:24 |
| salgado | 66- 'BZR_PLUGIN_PATH': get_bzr_plugins_path()}, | 19:24 |
| salgado | 67+ 'BZR_PLUGIN_PATH': get_BZR_PLUGIN_PATH_for_subprocess()}, | 19:24 |
| salgado | maxb, can you also align the two dict keys in there too? | 19:24 |
| maxb | pull the 'BZR_SSH' down onto a new line? | 19:25 |
| salgado | yeah, that's what I'd do | 19:25 |
| maxb | "The '-site' token tells bzrlib not to include the 'site specific plugins directory' (which is usually something like /usr/lib/pythonX.Y/dist-packages/bzrlib/plugins/) in the plugin search path, which would be inappropriate for Launchpad, which may be using a bzr egg of an incompatible version." ? | 19:32 |
| salgado | sounds good to me | 19:34 |
| maxb | return self.run_bzr_subprocess(args, | 19:34 |
| maxb | env_changes={ | 19:34 |
| maxb | 'BZR_SSH': 'paramiko', | 19:34 |
| maxb | 'BZR_PLUGIN_PATH': get_BZR_PLUGIN_PATH_for_subprocess() | 19:34 |
| maxb | }, | 19:34 |
| maxb | allow_plugins=True, retcode=retcode) | 19:34 |
| maxb | Is that formatting acceptable? | 19:34 |
| salgado | it's not -- the two arguments to run_bzr_subprocess() should be aligned | 19:34 |
| maxb | return self.run_bzr_subprocess( | 19:36 |
| maxb | args, env_changes={ | 19:36 |
| maxb | ? | 19:36 |
| salgado | that works, yes | 19:36 |
| salgado | args, env_changes={ | 19:36 |
| salgado | 'BZR_SSH': 'paramiko', | 19:36 |
| salgado | 'BZR_PLUGIN_PATH': get_BZR_PLUGIN_PATH_for_subprocess() | 19:37 |
| maxb | ok, pushed | 19:39 |
| === salgado is now known as salgado-afk | ||
| EdwinGrubbs | sinzui: I didn't see an email from you on the mockups. Do you want to discuss that today or wait till Monday? | 19:46 |
| sinzui | EdwinGrubbs: sorry. I am still doing reviews. I do need to send an email because I need to collect my thoughts | 19:47 |
| === EdwinGrubbs is now known as Edwin-lunch | ||
| bac | sinzui: thanks for catching the link issue. here is a screenshot of the revision. | 19:52 |
| bac | http://people.canonical.com/~bac/link-portlet-fixed.png | 19:52 |
| sinzui | bac: That is lovely | 19:53 |
| bac | i think that's a bit of a stretch but i'll take it and run off to ec2 | 19:54 |
| === henninge is now known as henninge-afk | ||
| === salgado-afk changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: -, - || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| sinzui | bac: Edwin-lunch: I have spent 7.5 hours doing just UI reviews and proposals. I am burned out. | 21:24 |
| bac | sinzui: and you should be burned out | 21:24 |
| === Edwin-lunch is now known as EdwinGrubbs | ||
| EdwinGrubbs | sinzui: well, I hope you enjoy your weekend. | 21:35 |
| sinzui | I wont. I am told I am attending multi-cultural night at my daughter's school and there will not be any food from asian :( | 21:36 |
| sinzui | My youngest daughter caught my wife playing tooth faerie. I need to hack her computer to destroy the evidence | 21:37 |
| sinzui | This same daughter just dressed the kittens in doll clothes | 21:37 |
| sinzui | My son who must has Asbergers just found all the puzzles I hid and is assembling them in my bedroom | 21:38 |
| bac | sinzui: smuggle in some sriracha and put it on everything | 22:10 |
| sinzui | well, I think that is an excellent idea. I will report back | 22:11 |
| rockstar | Can anyone do a really simple review for me? | 22:11 |
| rockstar | bac? ^^ | 22:15 |
| bac | rockstar: yes. i'm a very simple reviewer. | 22:15 |
| rockstar | bac, creating mp now. | 22:16 |
| rockstar | bac, https://code.edge.launchpad.net/~rockstar/launchpad/js-no-bigger-than-512k/+merge/19751 | 22:26 |
| * bac looking | 22:30 | |
| bac | hi rockstar | 22:33 |
| rockstar | bac, hi. | 22:33 |
| bac | it looks good but i have three comments | 22:33 |
| rockstar | bac, shoot. | 22:33 |
| bac | 1 - add #!/usr/bin/python | 22:33 |
| rockstar | bac, ah, yes. | 22:33 |
| rockstar | Shouldn't it be python2.5 ? | 22:34 |
| bac | 2 - dude, it's mid-february, start using 2010 already | 22:34 |
| rockstar | Ugh, c-n-p error. | 22:34 |
| bac | 3 - should max size be 512 * 1024 ? | 22:34 |
| rockstar | bac, it could be, yeah. I didn't think it was that big of a deal, but I'll changed it. | 22:35 |
| bac | ok, cool | 22:35 |
| bac | i'll add those to the MP and approve it | 22:35 |
| rockstar | bac, fixes pushed. | 22:35 |
| bac | done | 22:37 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
