/srv/irclogs.ubuntu.com/2011/01/06/#launchpad-reviews.txt

=== matsubara-afk is now known as matsubara
=== allenap changed the topic of #launchpad-reviews to: On call: Edwin, allenap || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
allenapjcsackett: Do you fancy a review? 400 lines, nothing very controversial. https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-3/+merge/4534911:48
jcsackettallenap: sure, i can take a look at that. is it time sensitive? still eating breakfast. :-)12:20
jcsackettallenap: looks like all your activereviews already have comments--safe bet someone has already gotten to the one you needed?12:23
jcsackettnm; i see those are both screenshots from you. which MP did you need?12:24
allenapjcsackett: No rush, https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-3/+merge/4534912:51
allenapThank you!12:51
gmballenap: I have a small JS branch for you if you're initerested.13:12
gmbOr indeed interested.13:12
allenapgmb: I'm doing one of your js branches already.13:12
=== allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: gmb || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmballenap: Ah, cool. Since I've only got one in the queue, it's likely to be the same one. :)13:13
=== allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
allenapgmb: And I just finished it :)13:13
gmbHurrah for small units of work.13:14
gmballenap: Ta for the review. I've made the necessary changes.13:19
allenapgmb: Cool.13:19
leonardrallenap, I have a branch to fix the recent buildbot failures13:55
leonardrhttps://code.launchpad.net/~leonardr/launchpadlib/correct-test-failure/+merge/4537013:55
leonardrfailure; https://lpbuildbot.canonical.com/builders/lucid_lp/builds/496/steps/shell_6/logs/summary13:55
=== jcsackett changed the topic of #launchpad-reviews to: On call: allenap, jcsackett || reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== gary-afk is now known as gary_poster
leonardrjcsackett: -^14:01
jcsackettleonardr: i've looked over it and it looks good. i'm requesting follow up from sinzui, since i'm being mentored.14:06
leonardrok14:06
leonardrjcsackett: there's also going to be a launchpad branch which just bumps up the launchpadlib used from 1.9.0 to 1.9.114:07
jcsacketti'll be happy to look at that whenever, since that will be an easy one. it may be a candidate for self review, however (not sure if we formally started doing that).14:08
jcsackettleonardr ^14:08
stubleonardr: I need the testsuite fix I landed today used by Launchpad soon.14:08
stub(launchpadlib testsuite that is)14:08
leonardrstub: ok, did you already do this work?14:09
stubleonardr: Yes, landed today. One line change to a doctest.14:09
leonardrah, that is a different one line change to a doctest14:09
leonardrwell, it'll go in to 1.9.1 as well14:10
stubCool. Once launchpadlib tests stop assuming the Librarian is running on port 58000 the sooner I can land this Librarian branch.14:10
=== matsubara is now known as matsubara-lunch
leonardrjcsackett: here's the launchpad branch: https://code.launchpad.net/~leonardr/launchpadlib/launchpad-integration/+merge/4538114:20
jcsackettleonardr: r=me. request for sinzui to follow up already made.14:22
leonardrthanks14:22
jcsackettnp. i like those one line reviews. :-P14:22
gary_postersinzui: ping.  I'm trying to speed up leonardr's testfix review, so I'd like your permission to approve jcsackett's reviews14:28
sinzuigo ahead14:30
gary_posterthank you14:32
gary_posterapproved14:32
=== jcsackett changed the topic of #launchpad-reviews to: On call: allenap, jcsackett || reviewing: -,allenap || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== jcsackett changed the topic of #launchpad-reviews to: On call: allenap, jcsackett || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jcsackettallenap: some questions and comments on your MP.14:57
allenapjcsackett: Cheers, thanks.14:57
=== salgado is now known as salgado-lunch
=== matsubara-lunch is now known as matsubara
bachi allenap, could you do a bugs review for me?16:02
allenapbac: Sure.16:02
bachttps://code.launchpad.net/~bac/launchpad/bug-5927/+merge/4540316:02
bacthanks allenap16:02
=== allenap changed the topic of #launchpad-reviews to: On call: allenap, jcsackett || reviewing: bac, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
sinzuisalgado, do you want to do a UI review of https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-3/+merge/4534916:06
=== salgado-lunch is now known as salgado
salgadosinzui, sure!16:22
bacthanks allenap16:45
allenapbac: Welcome :)16:45
=== allenap changed the topic of #launchpad-reviews to: On call: allenap, jcsackett || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
allenapabentley: Can you have a look at https://code.launchpad.net/~leonardr/launchpad/publish-tokens-2/+merge/34123. I don't know if it's still relevant, but it might warrant an updated vote from you.16:47
leonardrabentley, allenap: that code is landed, though possibly not in that branch16:48
allenapleonardr: Awesome, okay.16:49
abentleyleonardr, ACK.16:49
leonardrabentley, allena: actually, what happened was we decided to take an alternate approach16:50
abentleyleonardr, please mark the proposal rejected, then.16:50
leonardri'll mark the branch as abandoned16:50
salgadoallenap, I'm reviewing the UI of your subscription filter branch and am wondering what they're for...16:53
allenapsalgado: Hehe :)16:54
allenapsalgado: They are a new feature, so you can subscribe to a subset of bug mail.16:54
allenapsalgado: So you could subscribe to only mail for a particular tag, or set of statuses.16:54
salgadoallenap, ok, and when you want to subscribe to everything then there are no filters -- just the usual structural subscription?16:56
allenapsalgado: Yes, exactly like that.16:56
salgadoallenap, but as soon as you add a filter, you only get what matches that filter?16:56
allenapsalgado: Yes, and if there are multiple filters for a subscription they are ORed together.16:58
allenapsalgado: So you could have a filter for Fix Committed bugjam2010 bugs, another for open Critical bugs, and you'd get mail for both.16:59
salgadoright16:59
salgadoallenap, I'm wondering if it'd make sense to add a sentence to each subscription with filters stating that the user will only get email for bugs matching at least one of the filters?  that was not clear to me at first17:05
allenapsalgado: Okay, that makes sense. I can move the repeating "Bug mail filter" bit up and add an explanation.17:05
salgadoallenap, or maybe it could be at the top of the page so that we don't repeat it when there are multiple subscriptions with filters?17:07
allenapsalgado: Okay, that's cool too. Do you know of an example I can crib?17:08
salgadoallenap, you mean just the text?  I'll think of something17:10
allenapsalgado: Yeah, thanks.17:10
salgadoallenap, are structural subscriptions used for anything other than bug mail?17:14
allenapsalgado: Blueprints, in theory.17:14
salgadoallenap, hmm, then I think it'd be weird to talk about bugmail at the top when the subscriptions are not just for bugs.  /me thinks of something else17:20
allenapsalgado: I have an idea... I'll get a screen-grab for you.17:21
salgadoallenap, great!17:21
allenapsalgado: http://people.canonical.com/~gavin/ui/sub-search-ui-bug-656823-3/+structural-subscriptions,%20v2.png17:22
=== benji is now known as benji-lunch
salgadoI like that17:26
allenapsalgado: \o/ Cool.17:26
salgadoallenap, do you think it's worth to mention before the filters that messages that match any of them will go through?17:27
salgadoto make it clear that it's not just messages that match all of them that go through17:28
allenapsalgado: Mmm. I'm on the fence about that. I'll scratch my head for a few minutes. Maybe there's a clearer way to denote that.17:30
salgadoI'm not very fond of that either, but I don't think it's 100% clear that the filters are ORed together17:32
allenapsalgado: I have to go now, but I'll try to come up with something later.17:40
allenapThank you :)17:40
=== allenap changed the topic of #launchpad-reviews to: On call: jcsackett || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
salgadoallenap, np. I'll suggest something on the MP and maybe sinzui will have other ideas as well17:40
=== henninge changed the topic of #launchpad-reviews to: On call: jcsackett || reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
henningejcsackett: Hi! ;)18:08
jcsacketthenninge, hello!18:08
jcsacketti'm going to guess i know what you need reviewed. :-)18:09
henningeNo need to guess ...18:09
henningehttps://code.launchpad.net/~henninge/launchpad/db-devel-697845-import-failure/+merge/4542218:09
henninge:)18:09
=== benji-lunch is now known as benji
jcsacketthenninge, it's sort of frightening when an MP begins "Boy, how do I explain this..." :-P18:09
henningejcsackett: well, with message sharing and upstream-ubuntu sharing, translations is acquiring soyuz qualities when it comes to domain knowledge.18:10
henningeSome will argue it has always been that way ...18:11
jcsackettlol.18:11
jcsacketthenninge: so if i understand your notes, the new approach here aims to be more inclusive, rather than the very restrictive approach before?18:17
henningeexactly18:17
henningejcsackett: old, basic message sharing shared translations between all series within a distribution source package or between all series in a proudct.18:19
henningejcsackett: the new approach links those two sides using the packaging links.18:19
jcsacketthenninge: okay.18:19
henningejcsackett: "new" is "recife"18:20
jcsacketthenninge: right.18:20
henningejcsackett: but it must retain the old notion of "all series within a distribution are sharing translations"18:21
henningeand same for product18:21
henningejcsackett: but the first implementation of this did not do that but would exclude a distroseries if its sourcepackage was linked to a different product than the others series18:22
henningebut we have those situations in our database18:23
* jcsackett nods.18:23
jcsacketti can follow that.18:23
henningejcsackett: so, as you stated correctly, this approach now looks at all series of distribution source package and the products that the packaging links point to.18:24
henningelooking from the other side its "all series of a product and the source packages they are linked to".18:24
henningethis is the old sharing behavior *plus* templates from the "other side", not a completely new behavior.18:25
henningejcsackett: EOT ;)18:25
jcsackett:-)18:25
abentleyjcsackett, Could I interest you in reviewing https://code.launchpad.net/~abentley/launchpad/stale-targets/+merge/45418 ?18:27
jcsackettabentley, i would be happy to, right after i finish henninge's.18:27
=== jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || reviewing: henninge || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
abentleyjcsackett, tx18:27
jcsacketthenninge, i gather the somewhat frightening subquery in _queryBySourcePackageName is the searching via packaging links?18:29
henningeyes ;)18:29
jcsackettcool. as big as that is, it's still not terrible to understand.18:29
henningeit's the same in _queryByProduct, only that the subquery has to return a tuple.18:31
henningedistribution source pacakges are represented by the tuple (distribution, sourcepackagename)18:31
henninge"all packages with that name in any series of this distribution"18:31
jcsacketthenninge: i'm verifying all tests pass, but as far as read through of the code, this looks good. as soon as tests pass i will mark approved and get sinzui to follow up.18:36
jcsackettthanks very much.18:37
henningejcsackett: Great, thank! ;) I have to leave now but will come back later to land it.18:37
henninges18:37
jcsackettsounds good. :-)18:37
henningegood bye!18:46
=== jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || reviewing: abentley || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jcsackettabentley: is there anything test wise to make certain the widget is showing the things it should not just showing the things it shouldn't?18:52
jcsacketti could see someone breaking the widget so it shows nothing and the test still passing, because its only affirming that something isn't there.18:53
abentleyjcsackett, Yes, there are pagetests that cover the existing functionality.18:53
jcsackettabentley: ah, okay.18:53
jcsackettabentley: r=me. i have requested follow up from sinzui.19:18
abentleyjcsackett, cool, thanks.19:18
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
jcsackettis there anyone who could review something for me? https://code.launchpad.net/~jcsackett/launchpad/merge-ppas-697685/+merge/4545021:50
=== gary_poster is now known as gary-afk
bachi lifeless, does this look about right (re: bug 5927) ?  http://pastebin.ubuntu.com/551305/23:56
_mup_Bug #5927: assignee cannot see private bug <disclosure> <lp-bugs> <Launchpad itself:Triaged by bac> < https://launchpad.net/bugs/5927 >23:56

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!