/srv/irclogs.ubuntu.com/2010/01/25/#launchpad-reviews.txt

=== jamalta-afk is now known as jamalta
=== jamalta is now known as jamalta-afk
=== jamalta-afk is now known as jamalta
=== henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: - || queue [jpds] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara-afk is now known as matsubara
=== henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
* henninge can't stand the quiet .... ;)12:32
=== henninge 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
=== sinzui changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
gary_posterhey gmb.  For the release-critical request for https://code.edge.launchpad.net/~gmb/launchpad/remove-bug-heat-from-garbo-daily-bug-511283/+merge/17912 , I'm trying for the guideline of "if it would have been a CP, it can be a release-critical".  The problem you say it addresses does sound like it would be a CP candidate under non-release circumstances, but could you verify before I pursue further?14:05
sinzuigary_poster: I have a bug that I just tagged as a release blocker. Maybe I was wrong: https://bugs.edge.launchpad.net/launchpad-registry/+bug/51218214:21
mupBug #512182: Karma and bug activity missing when bugs are updated by creating a release <current-rollout-blocker> <Launchpad Registry:In Progress by sinzui> <https://launchpad.net/bugs/512182>14:21
gary_posterlooking14:22
gary_postersinzui: my evaluation is that it is CP candidate, and therefore I will approve it for RC if it is available, but it doesn't sound like a release blocker to me because the two symptoms are not critical (that is, I value an on-time release higher than those two symptoms).  Are you OK with that evaluation, or would you like to talk about it more?14:25
sinzuigary_poster: yes14:25
gary_postersinzui: cool thanks14:25
sinzuiEdwinGrubbs: can you review my branch at https://code.edge.launchpad.net/~sinzui/launchpad/release-bugtask-bug-512182/+merge/1798914:47
EdwinGrubbssinzui: sure14:47
EdwinGrubbssinzui: is it really 13,000 lines, or should it be merged into db-devel instead of devel?14:48
sinzuioh bugger14:48
sinzuiEdwinGrubbs: I will remake the merge request to db-devel, The diff is less that 100 lines14:49
EdwinGrubbsok, cool14:49
sinzuiEdwinGrubbs: https://code.edge.launchpad.net/~sinzui/launchpad/release-bugtask-bug-512182/+merge/1799914:54
=== salgado is now known as salgado-lunch
intellectronicaabentley: can i ask you to review a small branch of mine, an r-c candidate to fix those timeouts in bug listings?15:14
abentleyintellectronica: okay15:14
intellectronicaabentley: thanks. https://code.edge.launchpad.net/~intellectronica/launchpad/patch-badge-performance/+merge/1800015:14
abentleyintellectronica: BugTaskImageDisplayAPI already exposed has_patch?15:18
intellectronicaabentley: yes. i've added it in a previous branch, but didn't realise it's not being used because it's not included in the decorator15:18
abentleyintellectronica: What decorator do you mean?15:19
intellectronicai mean BugTaskListingItem15:19
abentleyintellectronica: I thought BugTaskImageDisplayAPI was an Interface because of its name and comment string.  Now I understand that BugTaskListingItemImageDisplayAPI is a subclass of it.15:30
abentleyintellectronica: I don't see any call site updates to match the new BugTaskListingItem.__init__.  Where is it constructed?15:35
abentleyintellectronica: never mind.15:36
intellectronicaabentley: found it? they're passed as a dict to BugListingBatchNavigator which is why you missed them15:36
=== matsubara is now known as matsubara-lunch
abentleyintellectronica: Okay, approved.15:38
intellectronicaabentley: thanks!15:38
intellectronicaabentley: can i ask for an extra line for review? the same needed to be added in milestone views, since they also use BugTaskListingItem. see http://pastebin.ubuntu.com/362666/15:42
abentleyintellectronica: r=me15:43
intellectronicacheers15:43
=== salgado-lunch is now known as salgado
=== matsubara-lunch is now known as matsubara
=== beuno is now known as beuno-lunch
sinzuiEdwinGrubbs: Do you have time for that review? Should I seek someone else?17:33
EdwinGrubbssinzui: I think this error is unrelated to your branch, but it is making it difficult for me to test things. When I run "make", I get ImportError: No module named builder.recipe17:34
EdwinGrubbssinzui: do you get that error?17:35
sinzuiI have not gotten that error.17:39
EdwinGrubbssinzui: I think rocketfuel-get will solve it. I've gotten so used to running "bzr up download-cache" that I hardly ever run it.17:40
sinzuiEdwinGrubbs:  I certainly would have noticed that error. I had trouble with the code...the test was right, but I forgot to use notify so I ran my test many times without issue to understand my incompetence.17:41
EdwinGrubbssinzui: make works now. Running the tests.17:41
EdwinGrubbssinzui: it looks like Bug.setStatus() calls BugTask.transitionToStatus() and sends an event. you could use that with bug.setStatus(bugtask.target, status, user)17:48
sinzuibugtask.editView uses the approach I took17:49
EdwinGrubbssinzui: ok, r=me17:51
sinzuiEdwinGrubbs: how would you test the use of bugtask.bug.setStatus(self.series_target, BugTaskStatus.FIXCOMMITTED, user)17:52
sinzuiCan I keep the same test and just update the code?17:52
EdwinGrubbssinzui: I think so.17:53
sinzuilet me try, The method you found has conjoined rules that I never ever want to write again17:53
sinzuiOh, I cannot17:56
sinzuiEdwinGrubbs: I need 5 more minutes, the target is not as obvious as I thought because of source packages17:56
sinzuiEdwinGrubbs: Thanks. I got rid of a lot of cruft: http://pastebin.ubuntu.com/362757/18:00
sinzuiThe test was fine18:01
EdwinGrubbssinzui: that looks good to go.18:02
sinzuithanks you very much. bug.setStatus works as I thought transitionToStatus() worked.18:02
=== matsubara is now known as matsubara-afk
sinzuiHi Gary. I am officially asking for an RC for this branch that we discussed a few hours ago: https://code.edge.launchpad.net/~sinzui/launchpad/release-bugtask-bug-512182/+merge/1799918:08
gary_poster_ack, sincui, approving.18:09
gary_poster_sinzui: 18:09
sinzuigary_poster: ^ If we landed it, you can create a release and all the foundations bugs will be updated for you for free!18:09
gary_poster_heh, cool18:09
=== beuno-lunch is now known as beuno
gary_poster_sinzui, the web interface is pretty slow, but I approved18:12
=== gary_poster_ is now known as gary_poster
=== henninge_ is now known as henninge
=== matsubara-afk is now known as matsubara
sinzuibeuno: Do you have time to do a follow up review of https://code.edge.launchpad.net/~sinzui/launchpad/needs-packaging-bug-509848/+merge/1784818:50
beunosinzui, yes18:50
bachi beuno19:13
beunohi bac 19:14
bacbeuno: on friday you noted some padding issues on the radio buttons as shown on http://people.canonical.com/~bac/linkup-multiple.png19:15
beunoyes19:15
bacbeuno: i'm just using a vertical LaunchpadRadioWidget...so i don't see how the padding on it could be different than elsewhere19:15
beunothose lines are all squished19:16
bacbeuno: but it does look cramped19:16
beunooff the top of my head, I can't think fo why that is19:16
baci don't know fo why either19:16
beunobac, are they in li's?  divs?19:17
beunoor just br's?19:17
bacbeuno: the widget does the rendering based on the vocab.  i haven't looked at the implementation of it to see how it is done.19:18
=== gary_poster is now known as gary-lunch
=== EdwinGrubbs is now known as Edwin-lunch
beunosinzui, doing your review19:52
beunoI think it's mostly ok19:52
leonardrgary: https://code.edge.launchpad.net/~leonardr/lazr.restful/generate-multiversion-operations/+merge/1803220:02
beunosinzui, done20:03
sinzuithank beuno20:04
sinzuibeuno: I can make the link changes. This changes will appears on all pages and in the page title20:06
sinzuis/this/those/20:07
beunosinzui, I think that's fine20:07
sinzuiI hope so, I only want one set of terms referring to these pages20:07
sinzuibeuno: That hardest part about developing a portlet to show the status of information that communities need to work with the porject is that we do not have a name for it20:08
beunosinzui, yaeh, it's more about "tell us how the community works"20:09
sinzuiThe information we show on the source package should be presented the same way on the project page: https://dev.launchpad.net/Registry/UbuntuLinkUpstream?action=AttachFile&do=get&target=linked-package.png20:10
sinzuiI have been looking for a term that uses contributor or community for project version of this portlet.20:11
sinzuiContributor Connections is the best term I have, and I do not like it20:11
beunohm20:12
beunoit's a tricky one20:12
sinzuioh, this information cooresponds to the Participation portlet, accept that it contains all the hosted and remote services20:14
beunoI'll think about it while I write a long email20:14
sinzuithanks20:14
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
bacbeuno: have a look at http://people.canonical.com/~bac/linkup-multiple-ff.png when you have a sec20:49
beunobac, much better20:51
beunobut the "Upstream project" label is too far from the question  :)20:51
bacbeuno: i didn't do anything.  :)  FF vs. webkit20:51
beunoyou could, of course, drop that label20:52
beunoit's meaningless to most people I think20:52
=== gary-lunch is now known as gary_poster
bacbeuno: that label is the 'title' for the widget.  i tried omitting it and just got a free-standing colon.20:57
beunogarrrrrrrr20:57
leonardrgary: if you want to warm up before reviewing my huge branch, here's an extremely short one:20:58
leonardrhttps://code.edge.launchpad.net/~leonardr/lazr.restful/missing-field-error/+merge/1803720:58
beunobac, can the label be the question?20:58
gary_poster:-) ok20:58
bacbeuno: if we want the question to end with ":"20:58
beunodouble garrrrrrrrrrrrrr20:58
bacbeuno: so the spacing issue is webkit only.  i propose to ignore it for this branch and find/file a bug to address it in a later branch20:59
beunobac, I'll stamp it if you target it   :)21:00
bacque?21:00
* bac curses his internet connection today21:01
beunobac, the bug for the webkit thingie21:01
bacbeuno: ok.  i'm still confused by stamp and target.  target where?21:02
bacsorry if i'm being dense21:02
beunobac, ah, sorry for confusing you21:02
beunoI'll approve the MP if you target the bug to be fixed in the next cycle21:02
beunothese are the kind of bugs that almost never get fixed  :)21:03
bacbeuno: sure i'll do that21:03
bacbeuno: but, of course, i'll mark it as foundations...21:04
beunoha21:04
beunowell played21:04
gary_poster:-P21:04
gary_posterleonardr: since you already looked at an earlier version of the file, could you review https://code.edge.launchpad.net/~gary/launchpad/bug_475371/+merge/18039 ?  I think this should just be comparing it to Tom's description in that bug report, as you already did, so I'm hopeful that this would be easy.21:53
leonardrgary, sure21:53
gary_posterthank you21:53
bacsinzui: my MP is up if you're interested:  https://code.edge.launchpad.net/~bac/launchpad/bug-490518-link-suggestion/+merge/1804021:55
sinzuibac: Thanks, I'll start it shortly21:56

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