=== jamalta_ is now known as jamal
=== jamal is now known as jamalta
=== beuno-on-vacatio is now known as beuni
=== beuni is now known as beuno
=== henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: []
=== danilos changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [danilo]
[10:30] <danilos> henninge, a lovely branch for you in the queue
[10:30]  * henninge already loves it ...
[10:30] <danilos> henninge, still waiting for MP to hit LP though
[10:35] <danilos> henninge, https://code.edge.launchpad.net/~danilo/launchpad/bug-408718/+merge/11682
[10:36] <danilos> henninge, btw, I couldn't see what you said with /me, it seems it's a bug in xchat
[10:36] <henninge> not important
[10:36] <henninge> danilos: can you never see it? like here:
[10:36]  * henninge thinks maybe its danilo and not xchat thats broken ...
[10:37] <danilos> henninge, exactly right there
[10:37] <adeuring> henninge: could you please review this mp: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-submissions-with-udev-data/+merge/11683 ?
[10:37] <henninge> danilos: so is it a bug in my xchat or yours.
[10:37] <henninge> adeuring: sure, put it in the queue, please.
[10:38] <danilos> henninge, mine, it's probably a translations bug
[10:38] <adeuring> henninge: thanks!
[10:38] <henninge> I see
=== adeuring changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [danilo, adeuring]
[10:38] <danilos> henninge, I've seen it before, a bad translation on messages with no c-format flag (or someone just cuts off %s)
[10:38] <henninge> mp page has a double title ...
=== henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: danilo || queue: [adeuring]
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: danilo || queue: [adeuring]
[10:41] <danilos> henninge, heh, no wonder it's a translations bug, just look at msgids: https://pastebin.canonical.com/22063/
[10:42] <henninge> what in the world ...
[10:44]  * danilos tests the non working version
[10:45] <danilo_> ok, it doesn't
[10:46] <henninge_> danilo_: doesn't what?
[10:46] <danilo_> henninge: don't worry, I am testing the bug in xchat-gnome; I'll move that to some other place, sorry :)
[10:50] <allenap> intellectronica: Fancy a pretty trivial ui conversion review? https://code.edge.launchpad.net/~allenap/launchpad/ui-convert-bugtarget-x-3.0-bug-427755/+merge/11685
[10:50] <intellectronica> allenap: sure
[10:51] <intellectronica> allenap: which pages should i look at?
[10:51] <allenap> intellectronica: Doh, I forgot to put that in. Sorry. I'll add a comment and ping you back.
[11:00] <henninge_> danilo_: I  do not see where the query is executed a second time after being aborted. Why return an empty result set?
[11:00] <danilos> henninge: I don't want to execute it a second time
[11:00] <danilos> henninge: as the cover letter explains, this is an optimization we can do without
[11:01] <danilos> henninge: if it fails, just do not do super-fast-imports but process that one pofile slowly
[11:01] <danilos> henninge: which brings up a good point: I should reset the statement_timeout to default after the query :)
=== henninge_ is now known as henninge
[11:07] <henninge> danilos: I must be missing some understanding of how superfast imports work ...
[11:07] <danilos> henninge, you might, but it's quite simple
[11:08] <danilos> henninge, basically, what they do is: fetch the entire pofile into ExistingPOFileInDatabase... if an identical message is already imported, don't run updateTranslation on it, and just skip over it
[11:09] <danilos> henninge, by disabling them in this way, it will be much slower when it happens, because updateTranslation will be called on each of the messages; however, updateTranslation is able to cope with identical messages anyway, so no harm done
[11:09] <danilos> henninge, and, if this keeps happening once a month on every file, no big deal either
[11:10] <henninge> danilos: so the rows fetched by _fetchDBRows are used for the check of what's already there
[11:10] <danilos> henninge, if we have time, we can log the query plan later this week (so stub has something to look at when it happens again)
[11:10] <danilos> henninge, exactly
[11:10] <allenap> intellectronica: I've added a comment. In most cases there's not a lot to see, except to see that it's sane.
[11:10] <henninge> danilos: ok, got it.
[11:11] <danilos> henninge, in essence, this is avoiding what is most likely a PG bug (the query that caused DB disk space to be eated returned ~1500 rows)
[11:12] <danilos> s/eated/eaten
[11:12] <intellectronica> allenap: sure, i just want to load the page, really, since we've seen in previous conversions that stuff got broken which wasn't covered by tests
[11:12] <intellectronica> allenap: thanks for adding the info
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: danilo, allenap || queue: [adeuring]
=== danilos changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: danilo, allenap || queue: [adeuring, danilo]
[11:20] <henninge> danilos: review sent.
[11:22] <henninge> danilos: forgot to sign it ...
[11:26] <danilos> henninge, about EmptyResultSet, note that this is SelectResults (from SQLObject emulation layer), so I went with the simplest thing possible (I don't know if there's EmptySelectResults emulation in Storm)
[11:27] <danilos> henninge, also, since this will set the statement_timeout to 5 minutes only for this run of the script (which should not be longer than 10 minutes), I won't touch that in order to keep the patch as simple as possible
[11:27] <henninge> danilos: I have used it lately, I think ...
[11:27] <danilos> henninge, with SQLObject code?
[11:27] <henninge> not sure ...
[11:27] <danilos> henninge, i.e. are you certain it has fetchall()?
[11:28] <henninge> danilos: no, I am not.
[11:28] <danilos> henninge, so, http://paste.ubuntu.com/270800/
[11:29] <henninge> danilos: I see. Too bad.
[11:29] <henninge> danilos: leave it as it is, then.
[11:30] <danilos> henninge, well, the alternative is to port the query to Storm, but I ain't doing that to re-enable poimport :)
[11:30] <henninge> no!
[11:30] <henninge> just get that done quickly ... ;)
[11:30] <beuno> intellectronica, care to take an easy-peasy review from me?
[11:31] <intellectronica> beuno: sure, let me just finish allenap's first
[11:31] <beuno> sure
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: danilo, allenap || queue: [adeuring, danilo, beuno]
[11:31] <beuno> intellectronica, https://code.edge.launchpad.net/~beuno/launchpad/margin-tweaks/+merge/11689
[11:50] <jtv> beuno, are you back already?  Hope you enjoyed it!  Just ran into your colon-lleno-de-galletitas translation while showing LP to a friend.
[11:51] <jtv> henninge, intellectronica: can I chuck a tiny little branch onto your queue?
[11:51] <beuno> jtv, I am, although I will be off again wed-fri  :)
[11:51] <jtv> beuno: good for you!
[11:52]  * jtv prepares to see "ui designer did not run from wed to sun" emails on the list
[11:52] <intellectronica> jtv: sure, add yourself to the queue. i'm just finishing allenap's and then there's a patchli from beuno and by then i'm sure one of us can pick up yours
=== jtv changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: danilo, allenap || queue: [adeuring, danilo, beuno, jtv]
[11:53] <jtv> intellectronica: thanks... patchli?  I like that...  been to Switzerland recently?
[11:54] <intellectronica> jtv: no, it's been too long. i miss the cheese
[11:54] <jtv> intellectronica: of the country's 3 main exports—clocks, cheese & banking—this is the only one with holes.
[11:55] <intellectronica> huh :)
[11:56] <intellectronica> allenap: r=me ui=me. let's take care of those two XXXs and please, give me back that sane human-readable formatting
[11:57] <allenap> intellectronica: Thanks :)
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: danilo, beuno || queue: [adeuring, danilo, jtv]
=== henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adeuring, beuno || queue: [danilo, jtv]
[11:59] <intellectronica> beuno: r=me
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adeuring, jtv || queue: [danilo]
[12:00] <intellectronica> jtv: 429272 ?
[12:00] <beuno> intellectronica, thanks. Do you know how sending through EC2 works now that it's part of the tree?
[12:01] <intellectronica> beuno: i didn't even know it's part of the tree. i use the same command i've always been using
[12:01] <jtv> intellectronica: yup
[12:01] <beuno> will do the same then
[12:01] <jtv> wooowww... a cloud just started falling down here
[12:02] <jtv> diagonally
[12:02] <intellectronica> jtv: can you give me some sample URLs to look at?
=== beuno is now known as beuno-lunch
[12:03] <jtv> intellectronica: whoops yes, of course.  Try 1) https://translations.launchpad.{dev,net}/+imports
[12:03] <jtv> 2) /~intellectronica/+imports
[12:03] <jtv> 3) uh...
[12:04] <jtv> https://translations.edge.launchpad.net/colon-full-of-cookie/trunk/+pots/colon-full-of-cookie/de/+filter?person=barry
[12:04] <jtv> vs local example: 
[12:04] <jtv> https://translations.launchpad.dev/alsa-utils/trunk/+pots/alsa-utils/es/+filter?person=carlos
[12:07] <danilos> intellectronica, thanks for skipping on my review request :(
[12:08] <intellectronica> omg, i never saw this project before, bizarre
[12:08] <intellectronica> danilos: you requested a review from me?
[12:08] <danilos> intellectronica, not from you, I've put it in the queue
[12:09] <danilos> intellectronica, for either of the two OCRs to pick it up (if it was only one person, I'd probably ping you, but with two?)
[12:09] <intellectronica> danilos: sorry, allenap, beuno and jtv personally asked me for a review and i took on their reviews in the order i promised them
[12:10] <intellectronica> danilos: in the future i recommend pinging as many people as you can. try to sound desperate and blonde. it helps
[12:10] <danilos> intellectronica, ok, I'll make a note to self to be much more "aggressive" in the future
[12:10] <intellectronica> but i'll also follow the queue more closely
[12:10] <danilos> intellectronica, thank you
[12:10] <henninge> adeuring: do you care to quickly explain to me where the files in your branch are used? I have no clue about what hwdb about.
[12:10] <henninge> I may have a guess, but ...
[12:11] <adeuring> henninge: the schema is used in l/c/l/scripts/hwdbsubmissions.py
[12:12] <henninge> adeuring: what does it do?
[12:12] <adeuring> henninge: it is used to ensure that data submitted by the HWDB client has "sane" data
[12:13] <adeuring> henninge: so that the processing script can make some aussptions that some data exists
[12:13] <henninge> adeuring: got your headset ready?
[12:13] <henninge> ;-)
[12:13] <adeuring> henninge: yes
[12:14] <intellectronica> jtv: r=me ui=me good stuff
[12:14] <jtv> intellectronica: thx
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adeuring, danilo || queue: []
[12:15] <intellectronica> jtv: i wonder how we can discover more pages like this. the test suite is unlikely to help, because it tests for the presence of the first title, not the absence of the second
[12:16] <jtv> intellectronica: we could have an automatic pagetest for double/omitted h1s I guess
[12:16] <intellectronica> danilos: 428440 ?
[12:17] <intellectronica> yeah, maybe that's something we should. there will be more pages like this
[12:17] <danilos> intellectronica, yep
[12:18] <intellectronica> let's talk to barry when he gets in
[12:20] <intellectronica> danilos: please add a comment between the condition and the then-clause of the if statement to distinguish them
[12:20] <danilos> intellectronica, sure
[12:21] <intellectronica> danilos: the indentation on line 27/28 is a bit off
[12:22] <stub> https://code.edge.launchpad.net/~stub/launchpad/kill-harder/+merge/11517
[12:22] <intellectronica> danilos: other than that it's all nice and good and r=me
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adeuring, - || queue: []
[12:23] <danilos> intellectronica, I don't see the problem with the indentation, what do you mean? (this is the default emacs python-mode does, and I never had complaints about it)
[12:23] <danilos> intellectronica, thanks for the review!
[12:23] <intellectronica> danilos: can't you see the the selves don't align?
[12:24] <danilos> intellectronica, they do for me (both on MP page and in my editor)
[12:24] <intellectronica> danilos: best is if you break immediately after the parenthesis
[12:24] <intellectronica> danilos: really?!
[12:24] <danilos> intellectronica, yeah
[12:25] <intellectronica> danilos: wow, looking with a text editor it indeed looks fine, but not in the mp. strange
[12:25] <intellectronica> maybe a chromium-related bug or something
[12:25] <danilos> intellectronica, perhaps some font issue then?
[12:25] <intellectronica> stub: so, is that a branch you want reviewed, or you were just testing your keyboard?
[12:25] <stub> That is a branch I'd like reviewed
[12:26] <intellectronica> danilos: yeah, but still, it's fixed-width, what could possibly go wrong
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adeuring, stub || queue: []
[12:26] <danilos> intellectronica, I've seen "fixed-width" fonts which aren't really (a slight mistake in metrics, and...)
=== henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, stub || queue: []
[12:29] <henninge> intellectronica: wow, you are quick ... ;-)
[12:30] <intellectronica> yeah, looks just fine in FF too
[12:31] <intellectronica> henninge: yeah, the trick is to only accept very small patches
[12:33] <intellectronica> stub: in line 117 it will be easier to read if you don't rely on coercion of the empty list to false and instead check for length
[12:34] <intellectronica> stub: what's the deal with the unspecified slice? is that your way of saying you want to clone the list so that you can modify it while iterating over the existing copy?
[12:35] <stub> yes - copy to be explicit I'm not modifying the source list.
[12:36]  * stub fixes line 117
[12:36] <intellectronica> stub: cool. r=me
[12:37] <stub> ta :-)
[12:37]  * stub adds a comment that pids is getting mutated.
[12:38]  * henninge lunches
=== henninge is now known as henninge-lunch
=== beuno-lunch is now known as beuno
=== mrevell is now known as mrevell-lunch
=== henninge-lunch is now known as henninge
=== bigjools-afk is now known as bigjools
[14:07] <adeuring> henninge: could you do a follow-up review of my previous MP: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-submissions-with-udev-data-2/+merge/11691 ?
[14:09] <henninge> yes, put it in the queue, I have something else to finish first.
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, - || queue: []
[14:10] <henninge> intellectronica: do you know of any random behavior of extract_text when used in a doc test (as opposed to pagetest)?
[14:11] <henninge> intellectronica: Ursinha observed that 
[14:11] <intellectronica> no, what's the random behaviour?
[14:11] <intellectronica> and what's the input?
[14:11] <henninge> Ursinha: ^
=== adeuring changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, stub || queue: [adeuring]
[14:12] <henninge> intellectronica: oh no, sorry. that is a pagetest
[14:13] <henninge> intellectronica: http://paste.ubuntu.com/270891/
[14:13] <henninge> Ursinha: how does it fail?
=== sinzui changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, stub || queue: [adeuring, sinzui, sinzui]
[14:14] <henninge> intellectronica: She said she needed lines 11 to 19 to get it to work.
[14:15] <henninge> intellectronica: but i just tried it without those lines and it works just fine.
[14:16] <intellectronica> what's the point of that complicated setup? can you not simply import extract_text?
[14:16] <intellectronica> heh, i should have asked about a million dollar
[14:17] <intellectronica> so what is it that doesn't work?
[14:17] <henninge> Ursinha: how did you come to know about this setup anyway? I didn't even know that was needed.
[14:17] <henninge> intellectronica: She seems to be no responding atm.
[14:17] <henninge> not
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, - || queue: [adeuring, sinzui, sinzui]
=== intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, sinzui || queue: [adeuring, sinzui]
=== henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adeuring, sinzui || queue: [sinzui]
[14:28] <Ursinha> henninge, bad connection, sorry
[14:29] <Ursinha> I said, I saw that in page-helpers.txt
[14:29]  * Ursinha checks the name of the file
[14:30] <Ursinha> henninge, actually that file didn't tell me to do that, but I tried to use it without importing stuff and it didn't work, so I looked in that file to see where did it come from
[14:31] <henninge> Ursinha: but extract_text is used all over Launchpad page tests so it should work.
[14:31] <henninge> Ursinha: how did it fail exactly?
[14:32] <bac> hey barry would you have time to do a 2nd UI review on some mailling list pages?  i've got pretty screenshots!
[14:32] <Ursinha> henninge, it said it couldn't find it
[14:32] <henninge> Ursinha: have you tried it again, just now?
[14:33] <Ursinha> henninge, running this moment
[14:33] <henninge> Ursinha: without the setup?
[14:33] <Ursinha> henninge, yes
[14:34] <Ursinha> henninge, and about the random behaviour, wasn
[14:35] <barry> bac: i would!  my stack is a bit deep right now, but i'm starting to pop items.  sign me up
[14:35] <Ursinha> wasn't'just with extract_text, but in simple output text, as I mentioned that happened with whitespaces it wanted me to put
[14:35] <Ursinha> henninge, now it ran without errors
[14:36] <bac> barry: i'll look around.  maybe post-vacation, wonderfully refreshed beuno has a spare moment for a UI review.  if not i'll hound you later.
[14:36] <henninge> Ursinha: ok, I'd say that was some confusion.
[14:36] <henninge> Ursinha: are you aware why it is better to use extract_text?
[14:37] <Ursinha> henninge, I don't think so
[14:37] <beuno> bac, I can take it!
[14:38] <henninge> henninge: First: If some-one decides in the future to put a link on one word in the test, you'd have to add extra ... for the test to pass.
[14:38] <henninge> with extract text, extra html is just filtered out.
[14:38] <henninge> Ursinha: ^
[14:38] <bac> beuno: intellectronica has already done a first UI review, so you're just mentoring:  https://code.edge.launchpad.net/~bac/launchpad/bug-421986-team-list-pages/+merge/11672
[14:38] <Ursinha> henninge, right
[14:39] <Ursinha> indeed
[14:39] <henninge> Ursinha: Second: If the test fails it outputs *all* text that is saw instead.
[14:39] <Ursinha> hmm
[14:39] <henninge> Ursinha: with html this can be *a lot* of text.
[14:40] <henninge> that makes reading the failure output of the text very, very hard.
[14:40] <henninge> I had that once ...
[14:40] <henninge> Ursinha: but with extract_text that html will get filtered out, too.
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui || queue: [sinzui]
[14:42] <intellectronica> sinzui: i find the quoted "codename" in a column header a bit funny
[14:42] <sinzui> intellectronica: it is consistent
[14:42] <intellectronica> sinzui: really? where else does it appear?
[14:42] <intellectronica> beuno: what do you think of it?
[14:43] <Ursinha> henninge, http://paste.ubuntu.com/270911/
=== mrevell-lunch is now known as mrevell
[14:43] <sinzui> We show the "codename" in quotes on several pages, series +index, +series, 
[14:44] <henninge> Ursinha: wonderful!
[14:44] <Ursinha> henninge, :)
[14:44] <henninge> Ursinha: saves me some typing I was just doing ... ;-)
[14:45] <henninge> Ursinha: r=me
[14:45] <beuno> intellectronica, I'm not crazy about it, although it is consistent, and I can't think of a much better way of doing it off the bat
[14:45] <Ursinha> henninge, many thanks
[14:45] <intellectronica> sinzui: i'd say we can remove the quotes, or just call the thing 'Version name'. i'm curious to hear what beuno and other people think about this
[14:46] <intellectronica> when i saw it, i thought the version is called 'codename', and it made it hard to understand what's going on. if it wasn't quoted it would have been better
[14:46] <beuno> intellectronica, is there a screenshot to look at?
[14:47] <intellectronica> these quotes are a literary device indicating irony, or other cases where the obvious meaning is not the correct parsing. i don't think it should be used in user interface much
[14:47] <intellectronica> beuno: let me prepare one for you
[14:48] <abentley> sinzui: It would be nice not to show the quotes when there's no codename.
[14:49] <intellectronica> beuno: https://devpad.canonical.com/~tom/version-codename-screenshot.png
[14:49] <adeuring> henninge: I'll leave quite soon -- if you have questions about my branch, can you ask them via mail?
[14:50] <sinzui> beuno: https://edge.launchpad.net/launchpad-registry/3.1
[14:50] <beuno> I don't feel the quotes are needed in the column header
[14:50] <sinzui> ^ abentley I do not see quotes being used
[14:50] <intellectronica> sinzui: in https://launchpad.dev/firefox/trunk/+milestones the table is screwed. there's one th missing, it seems
[14:50] <henninge> adeuring: I will, np.
[14:51]  * sinzui looks
[14:51] <abentley> sinzui: Neither do I.  Perhaps that's been fixed.
[14:51] <sinzui> abentley: the phantom quotes that appeared a few months ago was an ajax issue
[14:51] <intellectronica> sinzui: other than that ui=me
[14:53] <intellectronica> sinzui: and r=me as well
[14:54] <sinzui> I see the series column is missing.
[14:55] <beuno> bac, ball is in your court
[14:56] <bac> beuno: thanks!
[14:56]  * bac looks
[15:12] <Ursinha> henninge, the test suite run caused my notebook to freeze again
[15:13] <henninge> Ursinha: wow
[15:13] <henninge> Ursinha: Really the full suite or just this one page test?
[15:13] <Ursinha> henninge, it happens about 50% times I run it
[15:14] <Ursinha> henninge, I did bin/test -vvt lp.translations.*
[15:14] <henninge> Ursinha: interesting ...
[15:15] <henninge> Ursinha: what about running it in EC2?
[15:15] <Ursinha> henninge, that's ok, I just do that before submitting
[15:15] <henninge> Ursinha: ?
[15:15] <Ursinha> I mean, I run the translations tests
[15:16] <henninge> Ursinha: That must be a more general problem, though ...
=== danilos changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui || queue: [sinzui, danilo]
[15:17] <Ursinha> henninge, I think so, will try to do more research on it later
[15:18] <henninge> Ursinha: yeah, run it through ec2test for now ...
[15:18] <danilos> henninge, intellectronica, abentley: another branch in the queue, when you feel like it, I'd be more than happy to have it reviewed :)
=== bigjools changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui || queue: [sinzui, danilo, bigjools (1 line change!)]
[15:20] <abentley> intellectronica: Which sinzui are you reviewing?
[15:20] <Ursinha> henninge, yeah... the freezing tests aren't stopping me to land my branch, I just wanted to report
[15:22] <intellectronica> abentley: the one i claimed :)
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui, sinzui || queue: [danilo, bigjools (1 line change!)]
[15:26] <abentley> sinzui: In lib/lp/registry/templates/product-distros.pt, you have "distrribution-series" instead of "distribution-series".
[15:26] <sinzui> yuck
[15:31] <barry> henninge, intellectronica, abentley do any of you have time for a 178 line review?
[15:31] <abentley> barry: Sure.
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui, sinzui || queue: [danilo, bigjools (1 line change!), barry]
[15:32] <barry> abentley: awesome, thanks.  will mp it now
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui, bigjools || queue: [danilo, barry]
=== abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adeuring, sinzui, danilo || queue: [barry]
[15:34] <henninge> bigjools: now you even got two reviews for one line ;-)
=== henninge changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: sinzui, danilo || queue: [barry]
[15:35] <bigjools> henninge: I feel lucky
[15:37] <intellectronica> sinzui, beuno: so, any decision on the "codename"?
[15:38] <sinzui> I believe we remove quotes, we never show it is quotes
[15:38] <sinzui> s/is/in/
[15:40] <intellectronica> sinzui: i'm not sure i understand. you're going to remove the quotes from the column header?
=== abentley changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: sinzui, - || queue: [barry]
[15:40] <sinzui> I am removing it form the column header
[15:41] <sinzui> beuno: I don't feel the quotes are needed in the column header
[15:41] <sinzui> ^ beuno has spoken
[15:41] <sinzui> sorry, that quote does not look like a quote, but it is a quote
[15:44] <barry> abentley: mp sent
[15:45] <abentley> barry: Did you specify me as a reviewer?
[15:45] <barry> abentley: i did
[15:45] <abentley> barry: Okay, I'll review it when I get the email.
[15:46] <barry> abentley: thanks. it should be easy
[15:51] <bac> beuno: re: your question about breadcrumbs...  the url https://launchpad.dev/~testing-spanish-team/+invitation/hwdb-team only has as single breadcrumb for the testing-spanish-team.  the rest of the traversal does not generate any.  if the breadcrumb list only has one element they are not displayed.
=== abentley changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: sinzui, barry || queue: []
[15:52] <bac> beuno: so that's the explanation of the current behavior.  what do you think the breadcrumb for that URL should be?
[15:52] <beuno> bac, good question
[15:52] <beuno> I'm in a call, waiting to go into another call
[15:52] <beuno> so I'd say land it for now  :)
[15:52] <bac> ok, i'll answer in the MP.  we can revisit as necessary.
[15:53] <abentley> barry: You know that you can run lint anytime, right?
[15:53] <barry> abentley: i always remember that after bzr send tells me there are lint problems ;)
[15:53] <abentley> barry: Ah, okay.
[15:54] <barry> abentley: if we didn't have so many fp's i'd say bzr send should barf if there are lint problems
[15:55] <abentley> barry: I know the feeling.  Does your comment somewhat mean "I wish lint showed up earlier in the email"?
[15:55] <bac> beuno:  when you get a chance, please update your vote on https://code.edge.launchpad.net/~bac/launchpad/bug-421986-team-list-pages/+merge/11672
[15:56] <sinzui> barry: when you landed you title branch, did you write tests that verified titles like `+series : Mozilla Firefox`?
[15:56] <beuno> bac, sure. As soon as I get DNS back
[15:56] <barry> abentley: that would partly help because i wouldn't spend so much time writing the cover letter if i know i had lint to pick.  maybe also it could print the lint problems and then ask to continue or abort?
[15:56] <barry> sinzui: there are a billion tests for that.  every story prints browser.title.
[15:57] <abentley> barry: We could do that on the commandline, sure.  I think we'd have to display the lint, so folks would know if it was fp's.
[15:57] <sinzui> barry: okay, I will not let this block me form landing branches
[15:57] <barry> sinzui: fixing all those is what took so damn long for that branch
[15:57] <barry> abentley: +1
=== abentley changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: sinzui, - || queue: []
=== matsubara is now known as matsubara-lunch
=== Ursinha is now known as Ursinha-lunch
=== bigjools changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: sinzui, - || queue: [bigjools]
[16:40] <bigjools> got a complicated-ish one when LP sees my MP mail, I'll be around to guide you through it whoever can look at it
[16:47] <barry> abentley: thanks!
[16:47] <abentley> barry: np
=== deryck is now known as deryck[lunch]
=== sinzui changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: sinzui, - || queue: [bigjools, sinzui]
[17:25] <intellectronica> abentley: if it's ok with you i'm going off call. it's getting late and i still want to do some hacking this evening
[17:27] <intellectronica> i'll take that to be the good old silent affirmative
=== intellectronica changed the topic of #launchpad-reviews to: on call: abentley || reviewing: sinzui, - || queue: [bigjools, sinzui]
=== Ursinha-lunch is now known as Ursinha
=== deryck[lunch] is now known as deryck
[17:54] <cprov> abentley: would you have time for another review ?
[17:55] <abentley> cprov: I expect so.
[17:55] <cprov> abentley: okay, thanks -> https://code.launchpad.net/~cprov/launchpad/bug-417159-build-ui-tweaks/+merge/11703
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [bigjools, sinzui]
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [bigjools, sinzui, cprov]
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: bigjools || queue: [sinzui, cprov]
[18:00] <abentley> bigjools: Could you please document the parameters of newBinaryPublication and newSourcePublication?
[18:01] <bigjools> abentley: yep
[18:01] <bigjools> I have to run out for a bit, I'll be back in an hour
[18:03] <abentley> bigjools: Why does newBinaryPublication ignore the status parameter?
[18:03] <abentley> bigjools: (newSourcePublication too)
[18:06] <cprov> abentley: ignoring the status seems to be the right thing to do, publishing records should always be created as PENDING. 'status' shouldn't be an argument.
[18:06] <abentley> cprov: That sounds fine to me.
[18:06] <cprov> abentley: callsites that need a different status should update the record explicitly (only needed by tests, IIRC)
[18:08] <abentley> cprov: It looks like you're duplicating the functionality of canonical.launchpad.scripts.tests.run_script.  If possible, use run_script instead.
[18:10] <cprov> abentley: I've not checked the diff, but yes, run_script() should be used instead of local subprocess call.
[18:10] <abentley> cprov: I meant bigjools.
[18:10] <abentley> bigjools: It looks like you're duplicating the functionality of canonical.launchpad.scripts.tests.run_script.  If possible, use run_script instead.
[18:11] <cprov> abentley: no worries, it wasn't an offense ;)
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: sinzui || queue: [cprov]
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: cprov || queue: []
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: []
[18:27] <abentley> cprov: I've approved the code.  Whether you think it needs UI review is up to you.
[18:27] <cprov> abentley: thank you.
[18:28] <cprov> abentley: I will go with ui=rs, probably, unless beuno has time ...
=== sinzui changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui]
[19:44] <sinzui> beuno: I need a UI review as well as a code review for https://code.edge.launchpad.net/~sinzui/launchpad/source-package-ui/+merge/11718
[19:46] <beuno> sinzui, sure
[19:46] <beuno> no h1?
[19:46] <sinzui> hmm
[19:46] <beuno> in general, it's nice
[19:46] <sinzui> your right. there is no onw
[19:47] <beuno> what is alsa-utils-blah?
[19:47] <beuno> binary? diff?
[19:47] <beuno> deb?
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: sinzui || queue: []
[19:47] <sinzui> OMG
[19:49] <sinzui> beuno: the <h1> would be: alsa-utils
[19:49] <beuno> also, empty relationships are kinda sucky
[19:49] <beuno> Urgency: Low Urgency
[19:49] <beuno> can we drop the extra urgency?
[19:49] <beuno> branch, can we have a bzr icon for "trunk"?
[19:50] <sinzui> beuno: I really do not know the answers to these questions. No one on my teams knows sourcepackages. And we have no time to do real design
[19:50] <beuno> ok, I'll just throw it out there
[19:50] <beuno> you do whatever you can  :)
[19:51] <beuno> edit icon next to the project instead of "change upstream link"?
[19:51] <sinzui> beuno: That would be an error
[19:52] <sinzui> beuno: Having looked at the link, I think you mean Branch, which is actually a productseries
[19:52] <beuno> sinzui, yes
[19:53] <beuno> sinzui, ui=me is yours
[19:53] <beuno> I'll take whatever you manage to change
[19:53] <sinzui> beuno: I don't think I can drop the redundant Urgency, it is in the Enum
[19:54] <beuno> sinzui, I suspected as much
[19:55] <sinzui> beuno: Should I really put a bzr icon next to a product series...maybe I should change the label to say series
[19:57] <abentley> sinzui: "The HTML page title" seems ambiguous to me.  It could be an HTML fragment which is a page title, or it could be the text to use in the HTML title element.
[19:58] <sinzui> abentley: yes, it is now ambiguous since barry's branch
[19:58] <sinzui> abentley: it is still required though. (I reported a bu)
[19:58] <sinzui> g
[19:58] <abentley> sinzui: No, I mean that the docstring is ambiguous.
[19:59] <sinzui> abentley: oh yes, that was true when I started the branch
[19:59] <beuno> sinzui, change the label
[19:59] <sinzui> abentley: It was designed to be <title>
[20:00] <sinzui> beuno: NO the page will fail. barry did not remove the page_title requirement
[20:00] <abentley> sinzui: All the instances of that docstring are in code that you added.
[20:00] <barry> that is a bug
[20:00] <sinzui> abentley: I'll remove them
[20:01] <barry> i may get to fixing that today, but we'll see
[20:01] <abentley> sinzui: Okay.
[20:03] <sinzui> abentley: I am making them simple attrs
[20:03] <sinzui>     page_title = 'Upstream links'
[20:03] <sinzui> which I hope will become the breadcrumb when salgado gets his branch finished
[20:04] <abentley> sinzui: Makes sense.  I did wonder why they were properties.
[20:04] <sinzui> They use to put the object's displayname in them but that disappears yetsterday
[20:07] <sinzui> beuno: story I was not saying no to your suggestion to change the label. I am bad at juggling conversations. I changed Branch => Series
=== danilos changed the topic of #launchpad-reviews to: on call: abentley || reviewing: sinzui || queue: [danilo]
[20:38] <danilos> abentley, another UI branch up if you've got time
=== sinzui changed the topic of #launchpad-reviews to: on call: abentley || reviewing:  || queue: [danilo]
[20:38] <abentley> danilo: sure.
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: danilo || queue: []
[20:39] <abentley> danilo: Where?
[20:40] <danilos> abentley, waiting for MP to show up
[20:41] <abentley> danilos: Okay
[20:41] <danilos> abentley, https://code.edge.launchpad.net/~danilo/launchpad/bug-429414/+merge/11729
[20:45] <danilos> abentley, apparently, my last branch got some test breakages (my local copy didn't have the latest code by barry when I did a full test), so this one fixes a few of the failing tests from that branch as well
[20:45] <danilos> abentley, mostly page titles
[20:45] <abentley> danilos: Cool.
[20:45] <abentley> danilos: r=me
[20:46] <danilos> abentley, thanks
[20:46] <abentley> danilos: np
[20:47] <barry> sinzui, beuno wanna take a quick look at the new ~team/+invitations page?
[20:47] <sinzui> barry: sure
[20:48] <barry> sinzui: http://people.canonical.com/~barry/invite.png and http://people.canonical.com/~barry/noinvites.png
[20:49] <sinzui> barry: what do you think about the edit icon in the last column
[20:50] <barry> sinzui: i don't like it much.  that's the way the page currently is, and i couldn't think of anything brilliant to put there.  have any ideas?
[20:50] <sinzui> no
[20:51] <barry> yeah.  that's why i left it that way ;)
[20:51] <sinzui> I presume the edit is the accept/decline edit page
[20:51] <barry> yep
[20:52]  * sinzui looks for bugs users may have reported
[20:55] <sinzui> barry: our 15 minutes are up, and no user has reported an issue about it. The page fine
[20:57] <barry> sinzui: fab, thanks
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: -|| queue: []
[21:23] <bac> beuno: since we can ui=rs for mechanical changes can we also seek a single UI review from a non-beuno if we want a second pair of eyes?  the full mentored UI review is a killer for simple changes.
=== abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: -|| queue: []
[22:04] <bac> sinzui: would you have time to do a code review on my branch?
[22:04] <sinzui> I do
[22:05] <bac> cool.  i'll add you to the MP
[22:14] <Ursinha> hi sinzui
[22:14] <sinzui> Hi Ursinha
[22:14] <Ursinha> (wrong channel :)
[22:16] <bac> sinzui: https://code.edge.launchpad.net/~bac/launchpad/bug-429455-team-pages/+merge/11739
[22:16] <bac> hi Ursinha
[22:16] <Ursinha> hi bac
[22:19] <bac> sinzui: i'm going to step out for a bit and will answer when i return.
[22:39] <bac> sinzui: thanks for the comments
=== matsubara is now known as matsubara-afk