| === EdwinGrubbs 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 | ||
| noodles775 | BjornT: Hi! Can you do a CP review for me? https://code.edge.launchpad.net/~michael.nelson/launchpad/499421-dont-grind-bm-to-a-halt/+merge/19846 | 08:06 |
|---|---|---|
| noodles775 | I've added QA info to the MP there, and Julian's request to have it CP'd is on the linked bug. | 08:07 |
| BjornT | noodles775: the bug that you linked to in the XXX is marked as fix released | 08:10 |
| noodles775 | Yes, we thought the issue had gone, but it's happened again twice in the last two days (we're uncertain of the cause, this CP is just to ensure the buildd-manager doesn't die when it happens). | 08:11 |
| * noodles775 re-opens the bug. | 08:11 | |
| noodles775 | ie. this CP isn't fixing the bug, just improving the situation when it occurs. | 08:11 |
| BjornT | noodles775: right. i just wanted to know more about the issue, and wasn't sure whether the bug status was wrong, or you linked to the wrong bug | 08:13 |
| BjornT | noodles775: changing the summary and description of the bug would be good as well. the way it reads now, that CP does fix the bug, since the build farm doesn't grind to a halt anymore | 08:14 |
| noodles775 | BjornT: Yep, I thought I should wait until it was CP'd before doing that, but I can do it now. | 08:15 |
| BjornT | noodles775: well, ideally there should be one bug for this CP, and one bug for the XXX. re-defining the meaning of bugs should be avoided, since if you go back to look at this CP later, you will get confused. | 08:16 |
| noodles775 | BjornT: True, so this bug is really the CP (ie. ensuring it doesn't grind to a halt), and I'll create another bug for the on-going issue (I'll have to update the XXX in devel, but that's easy enough). Would that work for you? | 08:19 |
| BjornT | noodles775: yeah, thanks | 08:20 |
| noodles775 | Thanks BjornT. | 08:39 |
| al-maisan | Hello noodles775, I prepared a branch that merely updates the sample data in db-devel, https://code.edge.launchpad.net/~al-maisan/launchpad/sampledata-update/+merge/20112 | 08:49 |
| al-maisan | Could you please take a look? | 08:49 |
| noodles775 | al-maisan: great! I'll be there soon... | 08:49 |
| al-maisan | noodles775: thanks! | 08:49 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | noodles775, can I add one to the queue? | 10:28 |
| noodles775 | salgado: please do! | 10:28 |
| === salgado changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: sinzui || queue [salgado] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | noodles775, thanks. I'm just wrapping it up and the mp should be ready soon | 10:29 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: salgado || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | hrm... no MP yet. | 10:54 |
| adeuring | noodles775: I have another comment on hiding text via a CSS rule in https://code.edge.launchpad.net/~adeuring/launchpad/bug-513382/+merge/19970 ;) | 10:59 |
| noodles775 | adeuring: ok, I'll take a look soon. | 11:00 |
| salgado | noodles775, the mp should be up now | 11:07 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: salgado || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | intellectronica: can I tempt you to a UI review (should be straight-forward): https://code.edge.launchpad.net/~michael.nelson/launchpad/444988-remove-reference-to-karmic-on-ppa-page/+merge/20114 | 11:32 |
| intellectronica | noodles775: sure, but first i have to finish a review i'm doing for deryck | 11:33 |
| noodles775 | intellectronica: thanks, there's no rush. | 11:33 |
| noodles775 | salgado: you mention this is a temporary thing, what will happen to all the account records that are created during the window before the refactoring of the Account table? | 11:50 |
| intellectronica | noodles775: i like your change. i think it really improves the experience. ui*=me. | 11:51 |
| salgado | noodles775, we're going to move the openid_identifiers to a separate table and then drop Account* | 11:51 |
| noodles775 | intellectronica: ta. | 11:51 |
| noodles775 | salgado: ok, so none of it will be needed. Great. | 11:52 |
| bigjools | noodles775: crack bash for you in the queue :) | 12:21 |
| noodles775 | niice :) | 12:21 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: salgado || queue [noodles775, bigjools] || 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[lunch] || reviewing: - || queue [noodles775, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === bigjools changed the topic of #launchpad-reviews to: on call: noodles775[lunch] || reviewing: - || queue [noodles775, bigjools*2] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| leonardr | bac, i'm ready to land a big launchpad branch but i just realized i never got anyone to sanity-check my changes to download-cache. can you do it? it should only take a minute | 13:23 |
| leonardr | http://paste.ubuntu.com/383669/ | 13:23 |
| * bac looks | 13:23 | |
| bac | hurrah for updating launchpadlib. i hope 1.5.5 is a real release. r=bac | 13:24 |
| bac | btw leonardr, have you played with thekorn's ilaunchpad-shell? it's pretty cool. | 13:25 |
| leonardr | bac: i haven't played with it. but 1.5.5 is a real release | 13:26 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: bigjools || queue [noodles775, bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | Hi sinzui, would you have time to do a second UI review for https://code.edge.launchpad.net/~michael.nelson/launchpad/444988-remove-reference-to-karmic-on-ppa-page/+merge/20114 ? | 13:55 |
| sinzui | noodles775: yes | 13:55 |
| noodles775 | Thanks. | 13:57 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: thumper || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | noodles775: adeuring: bac: are all of you aware of the invisible-link that that are links use to generate real text for text browsers, visually impaired users, spiders, and our own test browser? | 14:09 |
| adeuring | sinzui: well, bac said that basically | 14:10 |
| * adeuring needed this head-upo | 14:10 | |
| adeuring | erm, heads-up | 14:10 |
| sinzui | Our text checks in the test suite did not break when we switched to sprites because the text is still rendered | 14:10 |
| noodles775 | sinzui: nope... | 14:11 |
| adeuring | sinzui: but the point here is that we create new text | 14:11 |
| sinzui | adeuring: konquerer and webkit may want the format to be: | 14:12 |
| sinzui | <span class="sprite heat-3"> <span class="invissible-link">accessible test</span></span> | 14:12 |
| sinzui | adeuring: You will see similar markup if you visit a page with a disabled link, or even an edit icon next to a value | 14:13 |
| adeuring | sinzui: my point is that the invisible text for the heat icons will appera 50 times on bug search pages | 14:13 |
| adeuring | this makes a search for "heat" really confusing | 14:14 |
| sinzui | person icons an milestone icons also appear many times | 14:15 |
| adeuring | sinzui: right. But in this case, we _want_ to show the person's/milestone's name, don't we? | 14:16 |
| sinzui | adeuring: bug heat is not worthy of an exception given that subscribers are a greater example that we do not think is a problem | 14:16 |
| sinzui | and I want to know the heat of each item | 14:16 |
| === adiroiban changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: thumper || queue [noodles775,adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | sinzui: well, we don't want to remove the heat icon entirely. | 14:17 |
| adeuring | the question is about the right way to present it on the page | 14:17 |
| bac | sinzui: i think adeuring's point is a search for a name will rightly bring up pages where she is a subscriber. but a search for 'heat' will bring up every bug page since every bug page has a heat display and indexable invisible text | 14:20 |
| bac | hmm, is that true? | 14:21 |
| sinzui | that is true for the terms launchpad and bug | 14:21 |
| bac | ah, a stronger counter-point | 14:21 |
| adeuring | bac: you found another isuue with invisible text ;) my point was: Do a simple browser search (crtl-f for firefox) on a bug listing | 14:22 |
| sinzui | I can point you to a bug in blueprint where the complaint is that you cannt search for "user" and get a short list of blueprints about users | 14:22 |
| adeuring | bac: but thanks for finding the search engine issue! | 14:22 |
| * sinzui may have closed it | 14:22 | |
| bac | adeuring: if i created a new novel argument it is because i misunderstood your original. | 14:23 |
| sinzui | adeuring: you are engineering a exception which is costly to build and maintain. Need I remind everyone that the link/image exceptions in answers and bugs caused months of bug fixes! | 14:23 |
| adeuring | bac: that the best way how misunderstandung can out, right ;)? | 14:23 |
| adeuring | sinzui: can you explain a bit? | 14:24 |
| sinzui | adeuring: bac: we should do this the standard way, feel free to report an bug about a *universal* problem where ambient text from icons cause bad search results. | 14:24 |
| sinzui | adeuring: we choose sprites because page load times are the *most* common problem for our users. Other issue are not as important | 14:26 |
| sinzui | If we change our rules for icon/images again, someone needs to fix all the exceptions in the pages. We really do want to use our standards to ensure we can adapt quickly | 14:27 |
| adeuring | sinzui: OK, I understand this point. But tehre is a difference between images that are used for "decorative" purposes, like the bug icons with different colours on bug linstsing, and images like the heat icons. The incofrmation encoded in the colour of the heat icon is "importance", and that is shon in its own column. People icons are just a decoration for a person's name, which is displayed indenedently and delibrately. | 14:28 |
| adeuring | while the bug heat icon is unque (foer each row of a listing) | 14:29 |
| adeuring | tehre is not other element carrying theis data | 14:29 |
| sinzui | So is the bug status and badges | 14:29 |
| adeuring | so, we can completely drop the alt attribute of of a person icon without causing any accessibiliy problems | 14:29 |
| adeuring | or of a milestone | 14:30 |
| jtv | noodles775: got one for you as well | 14:31 |
| noodles775 | jtv: great, pop it in the queue (I did see it earlier and was planning to get to it when the queue emptied..., but it hasn't yet). | 14:32 |
| jtv | noodles775: ok, thanks | 14:32 |
| === jtv changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: thumper || queue [noodles775,adiroiban(bug-359180-take-2),jtv] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === henninge_ is now known as henninge | ||
| henninge | noodles775: is it ok if I add mine to the queue, too? | 14:41 |
| noodles775 | salgado: what was it that I need to add to be able to log in locally now? add testopenid.dev to /etc/hosts? | 14:41 |
| noodles775 | henninge: please do, I'm doubtful that I'll get to it, but the next OCR will :) | 14:42 |
| henninge | noodles775: ok, understood ;-) | 14:42 |
| * noodles775 hopes to have a second OCR person around next week! | 14:42 | |
| === henninge changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: thumper || queue [noodles775,adiroiban(bug-359180-take-2),jtv, henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | salgado: nm, that worked. | 14:44 |
| salgado | noodles775, yeah, just adding that to /etc/hosts should do it | 14:44 |
| salgado | noodles775, it's done by rocketfuel-setup as well, in case you use it | 14:44 |
| henninge | noodles775: who is doing your branch? ;) | 14:45 |
| noodles775 | henninge: no one... yet :) | 14:45 |
| henninge | noodles775: done, r=me ;) | 14:51 |
| === henninge changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: thumper || queue [adiroiban(bug-359180-take-2),jtv, henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | henninge: thankyou sir! | 14:54 |
| === matsubara is now known as matsubara-lunch | ||
| noodles775 | jtv: would it be safer to forbid the running of the script unless LPCONFIG was dev? | 15:14 |
| jtv | noodles775: it's not set by default, and I don't want to add _too_ much safety or it'll just break needlessly. | 15:15 |
| jtv | We have a belt; this is suspenders. | 15:15 |
| jtv | (One suspender is the id check, the other the config check) | 15:15 |
| === noodles775 changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [adiroiban(bug-359180-take-2), henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-lunch is now known as matsubara | ||
| jtv | noodles775: thanks for the review! | 15:43 |
| jtv | noodles775: I'm a bit worried though that being this strict would stop people from playing with customized configs. | 15:43 |
| jtv | I realize it's always a tradeoff, but there's also the "do we have a really large production-like table" test. | 15:43 |
| noodles775 | jtv: well, if they're playing with customized configs, they're not going to have a problem spending 10 seconds to adjust the script if they want to. | 15:44 |
| jtv | noodles775: true, but this script already has another check to ensure it's not running on production; how many scripts do we have (without ever any trouble) that are just as dangerous without anyone ever adding checks like this? | 15:45 |
| jtv | I mean, if I hadn't added the check, would the average reviewer have thought of adding it? | 15:45 |
| noodles775 | jtv: which scripts? make schema? | 15:45 |
| jtv | noodles775: I guess, though I've no idea whether that has any checks. | 15:46 |
| noodles775 | jtv: yeah, I'm not sure. All the other scripts that I can think of *add* info, not delete. | 15:46 |
| jtv | launchpad-database-setup | 15:46 |
| * noodles775 looks | 15:47 | |
| jtv | mock-code-import ("warning! run make schema first!") | 15:47 |
| jtv | We also have a script now, apparently, to remove private data. There may be more. | 15:48 |
| noodles775 | Yep, you're right. | 15:49 |
| jtv | So I don't want to spend my time guarding against the admin who accidentally goes through the rigmarole for running scripts against a production db, with the --force option added. | 15:51 |
| noodles775 | yeah, I agree. I was more worried about the situation where a person runs it against a smaller DB with a different config. | 15:52 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue [henninge] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | noodles775: there's a good chance that the script might fail, and I'd estimate the risk of them running "make schema" by accident considerably larger. | 15:53 |
| jtv | (from shell history, f'rinstance) | 15:53 |
| noodles775 | jtv: I just thought it would have been a simpler implementation, not more difficult (ie. LPCONFIG == 'development'), but yep, I'm updating the MP now. | 15:54 |
| jtv | noodles775: thanks for being open to my rants. :) | 15:54 |
| * jtv goes for a quick bite | 15:54 | |
| === salgado is now known as salgado-lunch | ||
| === jelmer changed the topic of #launchpad-reviews to: rockstar || reviewing: - || queue [henninge,jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === jelmer changed the topic of #launchpad-reviews to: ON CALL: rockstar || reviewing: - || queue [henninge,jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === jelmer changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue [henninge,jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jelmer | argh, sorry! | 16:16 |
| === bdmurray changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue [henninge,jelmer,bdmurray] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | Hi rockstar! | 16:23 |
| rockstar | henninge, hi | 16:23 |
| henninge | rockstar: I will have to leave on top of the hour. Do you think you can look at my branch soon, so I can answer any questions you might have? | 16:24 |
| rockstar | henninge, I'm in the LP production meeting. When that's over, I'll start my reviews for the day. | 16:24 |
| henninge | rockstar: great! ;-) | 16:25 |
| henninge | oh, it's still early in Colorado ... | 16:25 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: henninge || queue [jelmer,bdmurray] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| al-maisan | hello rockstar, could you please have a quick peek at this db-devel testfix branch: https://code.edge.launchpad.net/~al-maisan/launchpad/disable-mintime-tests/+merge/20143 ? | 16:31 |
| rockstar | al-maisan, r=me | 16:31 |
| al-maisan | rockstar: thanks! | 16:31 |
| al-maisan | rockstar: sorry for the bother but I overlooked one tests that needed to be disabled as well: http://pastebin.ubuntu.com/383778/ | 16:35 |
| rockstar | al-maisan, it's good, land it. | 16:35 |
| al-maisan | rockstar: good man! Thanks! | 16:35 |
| rockstar | henninge, did lp.translations.pottery.build_slave change to lp.translations.pottery.buildd in a previous branch? | 16:41 |
| henninge | no, that's in here. | 16:41 |
| henninge | rockstar: ^ | 16:41 |
| rockstar | henninge, I don't see it in the diff | 16:41 |
| henninge | rockstar: line 87 | 16:41 |
| * rockstar facepalms | 16:42 | |
| rockstar | Thanks. | 16:42 |
| rockstar | henninge, r=rockstar | 16:44 |
| rockstar | bdmurray, where's your merge proposal? | 16:44 |
| henninge | rockstar: cool, thanks | 16:44 |
| bdmurray | rockstar: well, it's actually been approved. I ran it through ec2test and it passed but it hasn't been merge so I'm uncertain if it is a permissions thing or something else | 16:45 |
| rockstar | bdmurray, who ran it through ec2 for you? | 16:45 |
| bdmurray | rockstar: I ran the ec2 test myself | 16:46 |
| rockstar | bdmurray, ah, and you don't have pqm access? | 16:46 |
| rockstar | bdmurray, who reviewed the branch? | 16:46 |
| bdmurray | rockstar: that's likely the problem - jtv reviewed it | 16:46 |
| bdmurray | https://code.edge.launchpad.net/~brian-murray/launchpad/api-export-person-logo_link/+merge/20074 | 16:46 |
| rockstar | bdmurray, usually, you'd want jtv to land it for you. | 16:46 |
| jtv | rockstar: I checked but he was in what I thought was the right team. bdmurray: I let you try it yourself first, but apparently that didn't work. Sorry for that; I can now land it for you as per yesterday's Plan B. | 16:48 |
| bdmurray | jtv: okay, thanks! | 16:48 |
| rockstar | jtv, you are awesome. | 16:52 |
| * jtv checks | 16:52 | |
| rockstar | jtv, you don't have to check. You are awesome. | 16:52 |
| rockstar | :) | 16:52 |
| rockstar | I thought you had EOD'd already, so was going to volunteer. | 16:52 |
| jtv | rockstar: you exaggerate, but pleasantly. But EC2 is firing up. | 16:53 |
| jtv | rockstar: I'm at fuzzy EOD. Kick off some landings first for a pleasant morning. :) | 16:53 |
| rockstar | jelmer, is this diff up to date? Does it have a dependent branch? | 16:53 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: jelmer || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jelmer | rockstar: the bug471148 one? That's current now | 16:54 |
| rockstar | jelmer, okay. | 16:55 |
| jelmer | rockstar: It's a fair bit smaller now that Muharems prerequisite branch has landed. | 16:55 |
| rockstar | jelmer, you should set the prerequisite branch on the mp anyway, because it's informational. | 16:56 |
| jelmer | rockstar: Even if that branch has already landed? | 16:56 |
| rockstar | jelmer, yeah, it's still a good idea to say "I based my work off of this work." | 16:57 |
| rockstar | Not a necessity, but for history's sake. | 16:57 |
| jelmer | rockstar: Ah, that makes sense. I'll keep that in mind for next time. | 16:57 |
| rockstar | jelmer, your tests need comments/documentation | 16:58 |
| rockstar | lib/lp/soyuz/tests/test_processor.py and lib/lp/soyuz/tests/test_publishing.py | 16:58 |
| rockstar | Actually, it looks like some of the tests have docstrings, so you'll probably just need to add the docstrings to the ones that lack them. | 16:59 |
| jelmer | ok | 17:00 |
| === sinzui changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: jelmer || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| rockstar | jelmer, good on you for adding factory methods for your new work. | 17:00 |
| sinzui | rockstar: bac: EdwinGrubbs: can one of you review my branch to fix a timeout: https://code.launchpad.net/~sinzui/launchpad/portlet-package-summary-timeout/+merge/20147 | 17:01 |
| rockstar | sinzui, I can when I'm done with jelmer. | 17:01 |
| jelmer | rockstar: Thanks | 17:02 |
| rockstar | jelmer, other than the missing docstrings, this looks like very good work. | 17:02 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jelmer | rockstar: Thanks for the review :-) | 17:08 |
| rockstar | sinzui, what specifically is the portlet you're talking about? | 17:13 |
| rockstar | sinzui, ? | 17:23 |
| sinzui | https://edge.launchpad.net/ubuntu/lucid <-- Upstream packaging | 17:24 |
| rockstar | sinzui, ah, okay, thanks. | 17:30 |
| rockstar | sinzui, did you cherry pick to staging or were you running raw queries? | 17:30 |
| sinzui | rockstar: I ran raw queries comparing the oops query to the modified one | 17:31 |
| gary_poster | bac, I have a gigantic diff for review (1984 lines), but the size mostly comes from the same mechanical change over and over again. You might be interested because it addresses bug 496705, which you entered after we talked. It also has some other changes that I think are generally interesting. If you are not interested; I'll try for a salespitch on rockstar. :-) | 17:31 |
| gary_poster | https://code.edge.launchpad.net/~gary/launchpad/bug491705/+merge/20156 | 17:31 |
| mup | Bug #496705: Single entry point for site-wide initialization needed <Launchpad Foundations:In Progress by gary> <https://launchpad.net/bugs/496705> | 17:31 |
| rockstar | gary_poster, at 1984 lines, it better be one hell of a salespitch. :) | 17:32 |
| rockstar | sinzui, okay. | 17:32 |
| gary_poster | rockstar: heh | 17:33 |
| rockstar | sinzui, this looks good. r=rockstar | 17:35 |
| sinzui | rockstar: thanks | 17:35 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| gary_poster | rockstar: um, so, 1600 lines are the following repetition: | 17:37 |
| gary_poster | -#!/usr/bin/python2.5 | 17:37 |
| gary_poster | 988+#!/usr/bin/python2.5 -S | 17:37 |
| gary_poster | :-D | 17:37 |
| gary_poster | please? | 17:38 |
| rockstar | gary_poster, wow. That's a lot of python. | 17:38 |
| gary_poster | oh yeah | 17:38 |
| === jelmer changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: || queue [jelmer/al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| gary_poster | bac, I'm now changing my begging to rockstar. rockstar, if you can't stand it, lemme know, and I'll go beg on the foundations channel. :-) | 17:40 |
| rockstar | gary_poster, looking at it now. It hasn't been too bad. | 17:40 |
| gary_poster | rockstar: thank you | 17:40 |
| rockstar | gary_poster, this seems to kill a lot of hacks we had. | 17:42 |
| rockstar | (hacks I didn't even know about) | 17:42 |
| gary_poster | rockstar: heh, yeah. that was a definite goal. :-) | 17:43 |
| rockstar | gary_poster, okay, cool. | 17:43 |
| rockstar | gary_poster, also, it looks like you killed a lot of trailing whitespace issues. Was that on purpose, or is your editor doing that? | 17:43 |
| gary_poster | rockstar: on purpose because I tell my editor to do it. :-) If you want me to separate I will. I felt it was relatively small, but I'm obviously not in much of a bargaining position there. ;-) | 17:44 |
| rockstar | gary_poster, well, it seems more effort to separate them than it's worth. | 17:45 |
| gary_poster | ack | 17:45 |
| gary_poster | thanks | 17:45 |
| rockstar | gary_poster, however, while that does need to be done, it can cause spurious conflicts, so do so sparingly in the future plz. | 17:45 |
| gary_poster | rockstar: ack, will do | 17:46 |
| rockstar | I'm mostly impressed with the changes in buildout-templates/_pythonpath.py.in | 17:46 |
| rockstar | You replaced 110 lines of code with 14 lines. :) | 17:47 |
| rockstar | gary_poster, these are pretty invasive changes, and we're getting nigh to closing PQM time. | 17:47 |
| rockstar | gary_poster, are you confident these changes aren't going to have explosive bugs? | 17:47 |
| gary_poster | rockstar: ack, see last para of cover letter. Going to insert as soon as branch opens. | 17:48 |
| rockstar | gary_poster, ah, hadn't seen that. | 17:48 |
| rockstar | gary_poster, r=rockstar | 17:53 |
| gary_poster | thanks rockstar! | 17:53 |
| rockstar | gary_poster, the diff was surprisingly deceptive of the actual changes. :) | 17:53 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: <no-one-because-rockstar-needs-a-break> || queue [jelmer/al-maisan] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| gary_poster | rockstar: yeah. :-) I tried to talk about em in the cover letter, but still | 17:54 |
| rockstar | gary_poster, the cover letter was actually more helpful than the diff in this case. :) | 17:54 |
| gary_poster | :-) good. thanks again. | 17:54 |
| === salgado-lunch is now known as salgado | ||
| === gary_poster is now known as gary-lunch | ||
| abentley | rockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/qa-ready/+merge/20162? | 18:28 |
| rockstar | abentley, sure. | 18:28 |
| === abentley changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: <no-one-because-rockstar-needs-a-break> || queue [jelmer/al-maisan/abentley] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| rockstar | abentley, I thought there was a page that had only the revno on it. Did that not pan out? | 18:30 |
| abentley | rockstar, I don't think there's any such page. | 18:30 |
| rockstar | abentley, okay. | 18:30 |
| rockstar | abentley, the use of a bzr transport to screen scrape confuses me. What does it provide that a regular urllib or something doesn't? | 18:32 |
| rockstar | Also, it sure would be nice if we had a successful-updates.txt for edge. | 18:33 |
| rockstar | (not your problem though) | 18:33 |
| abentley | rockstar, it respects all the configuration data that bzr does, so if you have proxies or things set up for bzr, those will apply to the transport also. | 18:33 |
| rockstar | abentley, ah, okay. | 18:33 |
| abentley | rockstar, also, it's familiar, and I don't see any reason *not* to use it. | 18:34 |
| rockstar | abentley, that's the reason I would have suspected, but the former is also a very good reason. | 18:34 |
| rockstar | s/suspected/expected/ | 18:34 |
| rockstar | abentley, was there a technical reason is_present appears after main? | 18:38 |
| abentley | rockstar, no. | 18:38 |
| rockstar | abentley, so, it's not a big deal, but I usually expect functions to be defined before I see the calls. It's still valid python, but hard to follow. Would you mind just bumping is_present above main? | 18:39 |
| abentley | rockstar, sure. | 18:40 |
| rockstar | abentley, r=rockstar | 18:42 |
| === deryck is now known as deryck[lunch] | ||
| abentley | rockstar, thanks. | 18:52 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: jelmer || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === gary-lunch is now known as gary_poster | ||
| rockstar | abentley, could I get you to do a small javascript branch? | 19:41 |
| rockstar | s/do/review | 19:41 |
| === deryck[lunch] is now known as deryck | ||
| rockstar | EdwinGrubbs, could I get you to take a look at a branch to fix a bug you filed? | 19:45 |
| EdwinGrubbs | rockstar: sure | 19:46 |
| rockstar | EdwinGrubbs, https://code.edge.launchpad.net/~rockstar/launchpad/code-js-reorg/+merge/20170 | 19:46 |
| rockstar | EdwinGrubbs, it's run the windmill gambit and everything is hunky dory. | 19:47 |
| rockstar | EdwinGrubbs, wait, it looks like I didn't get all the revisions I needed. Please hold for a new diff. | 19:49 |
| rockstar | EdwinGrubbs, new revision pushed, should update the diff in a few minutes. | 19:55 |
| === sinzui changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | rockstar: I have a short documentation fix: lp:~sinzui/launchpad/ds-get-source-package into lp:launchpad/devel | 19:59 |
| rockstar | sinzui, what's the mp url? | 19:59 |
| sinzui | rockstar: https://code.launchpad.net/~sinzui/launchpad/ds-get-source-package/+merge/20172 | 20:00 |
| rockstar | sinzui, easy. r=rockstar | 20:01 |
| sinzui | thanks | 20:01 |
| rockstar | EdwinGrubbs, howgoesit? | 20:06 |
| EdwinGrubbs | rockstar: I'm just starting on it now. | 20:10 |
| rockstar | EdwinGrubbs, great, thanks. | 20:10 |
| EdwinGrubbs | rockstar: the module for javascript/code/branchlinks.js should be "code.branchlinks". The only thing that needed to be changed was the namespace variable which you did. | 20:14 |
| rockstar | EdwinGrubbs, why should it be code.branchlinks? | 20:14 |
| EdwinGrubbs | rockstar: there already is a javascript/lp/ directory, so the files in there are in the lp module. | 20:15 |
| rockstar | EdwinGrubbs, :/ | 20:15 |
| rockstar | EdwinGrubbs, what happens when canonical/launchpad/javascript doesn't exist anymore? | 20:16 |
| rockstar | At least, canonical/launchpad/javascript/code won't exist much longer. | 20:16 |
| EdwinGrubbs | rockstar: well, lib/lp/code/javascript could be the new home for the "code" module. If you want lp to prefix everything, you will have to bring it up at the reviewer meeting. | 20:17 |
| rockstar | EdwinGrubbs, okay, I withdraw my proposal then, and I'll bring it up at the next reviewer meeting. | 20:18 |
| abentley | rockstar, sorry, was on lunch. Sounds like you got a better review anyhow. | 20:19 |
| EdwinGrubbs | rockstar: ok, sorry for the confusion. I thought the docs were clear. | 20:19 |
| * rockstar afks to the shop | 21:07 | |
| === salgado is now known as salgado-afk | ||
| === matsubara is now known as matsubara-afk | ||
| leonardr | rockstar, i'm leaving soon but if you get some free time today or tomorrow, would you take a look at https://code.edge.launchpad.net/~leonardr/lazr.restful/mutators-are-not-named-operations ? | 22:01 |
| === rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!