al-maisanGood morning 06:49
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
adeuringal-maisan: could you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/bug-457475-udev-device-id/+merge/13771 ?09:49
al-maisanadeuring: yep .. will get to it in a few minutes.09:50
adeuringal-maisan: thanks!09:50
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [abel] || This channel is logged: http://irclogs.ubuntu.com/
adeuringal-maisan: just noticed that I should add one more tests....09:54
al-maisanadeuring: that's fine .. please let me know when the branch is ready.09:55
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
adeuringal-maisan: sure. sorry for the mess....09:55
al-maisanno problem :)09:55
adeuringal-maisan: done. See the comment in the MP10:03
al-maisanadeuring: OK .. thanks.10:05
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
al-maisanadeuring: your branch is targeting db-devel .. is that right?10:07
adeuringal-maisan: I'm an idiot... the target should be devel...10:08
al-maisanadeuring: no worries .. just checking.10:08
al-maisanadeuring: so, udev devices do not get IDs reported for them and you are making up an ID by counting the udev devices in the report?10:18
=== danilo-afk is now known as danilos
al-maisanadeuring: how is that device_id for udev device used later on?10:18
adeuringal-maisan: yes. as written in the the MP, they are needed for a SQL table column, that they main purpose of that column is to help debugging the processing script. It makes it easy to figure out which device related to which record in the table10:19
al-maisanadeuring: is the scope of the device_id limited to the respective HWDB report?10:19
adeuringal-maisan: yes, the ID should be unique only within one submission10:19
al-maisanadeuring: I see, thanks10:20
adeuringal-maisan: I know that there should be a test how UdevDevice is used to populate the SQL tables, but there are qt least two more bugs that need fixing before I can do that.10:22
al-maisanadeuring: hmm..10:22
al-maisanadeuring: stupid question re.10:26
al-maisan        self.assertEqual(1, device.id)10:26
adeuringal-maisan: yes?10:26
al-maisanhow can you know that the id value is 110:26
al-maisanbecause it is the root/first device?10:26
adeuringah, the method setUp() defines self.root_device, and there is the dict item 'id': 110:27
adeuringal-maisan: the change of lines 635ff in test_hwdb_submission_parser.py10:27
adeuringal-maisan: I meant the change in 3036ff in test_hwdb_submission_processing.py10:28
adeuring(not my best day....)10:28
al-maisanadeuring: I see .. thanks10:28
al-maisanadeuring: r=me10:28
adeuringal-maisan: thanks!10:28
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
=== deryck is now known as deryck[afk]
=== matsubara-afk is now known as matsubara
=== mrevell is now known as mrevell-lunch
=== salgado-afk is now known as salgado
adeuringal-maisan: fancy another review? https://code.edge.launchpad.net/~adeuring/launchpad/bug-458029-kernel-package-name-for-udev-submissions/+merge/1378112:47
al-maisanadeuring: yes, just give me a few minutes.12:49
adeuringal-maisan: thanks!12:49
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [adeuring] || This channel is logged: http://irclogs.ubuntu.com/
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
=== mrevell-lunch is now known as mrevell
=== matsubara is now known as matsubara-afk
al-maisanadeuring: I need to take a short break and will look at your branch immediately after I return.13:46
adeuringal-maisan: sure, np13:47
=== matsubara-afk is now known as matsubara
=== gary_poster is now known as gary-sprint
al-maisanadeuring: looking at your branch now14:42
adeuringal-maisan: great!14:43
abentleyal-maisan: Could you please review https://code.launchpad.net/~abentley/launchpad/reply/+merge/13739 ?14:46
al-maisanabentley: as soon as I finish with adeuring's branch14:46
=== deryck[afk] is now known as deryck
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: adeuring || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/
abentleyal-maisan: Thanks!14:47
al-maisanyou are welcome14:47
=== danilos changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: adeuring || queue: [abentley, danilo] || This channel is logged: http://irclogs.ubuntu.com/
danilosal-maisan: hi, I hope you'll have enough time for another review after you are done with adeuring and abentley :)14:50
al-maisanhello danilos, it may be a bit tight but I should be able to squeeze it in hopefully ;)14:51
danilosal-maisan: cool, thanks :)14:59
al-maisanadeuring: r=me15:00
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [abentley, danilo] || This channel is logged: http://irclogs.ubuntu.com/
adeuringal-maisan: thanks!15:00
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: abentley || queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com/
al-maisanabentley: is this the MP in question: https://code.launchpad.net/~abentley/lp-production-configs/trunk/+merge/13783 ?15:03
al-maisanabentley: sorry15:04
al-maisanjust read your qustion above15:04
abentleyal-maisan: no, it's https://code.launchpad.net/~abentley/launchpad/reply/+merge/1373915:04
al-maisanabentley: r=me15:20
abentleyal-maisan: Thanks!15:20
abentleybarry: Could you do a ui review for me?  https://code.launchpad.net/~abentley/launchpad/reply/+merge/1373915:21
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com/
al-maisandanilos: is this the MP in question: https://code.edge.launchpad.net/~danilo/launchpad/bug-455771/+merge/13699 ?15:23
barryabentley: sure if you can give me screen shots.  i have no way to run devel branches right now15:23
abentleyal-maisan: Actually, this needs UI review still, so the status should still be work-in-progress.15:23
danilosal-maisan: no15:23
al-maisanabentley: OK .. sorry .. will revise it.15:23
abentleyal-maisan: I've already done.15:24
al-maisanabentley: thanks15:24
danilosal-maisan: it seems my emails are still not getting through to LP: https://code.edge.launchpad.net/~danilo/launchpad/bug-458036/+merge/1378715:25
al-maisandanilos: OK15:25
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: danilo || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
abentleybarry: Updated with screenshots.15:32
al-maisandanilos: looks good, r=me15:37
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
=== al-maisan changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
danilosal-maisan: thanks!15:37
al-maisandanilos: does your branch need an extra ui review as well?15:38
danilosal-maisan: I don't think so, I am just changing privileges on the page15:41
danilosal-maisan: it has been UI reviewed before by beuno15:41
al-maisandanilos: great .. I'll set it to approved then.15:41
danilosal-maisan: ta!15:42
al-maisanyou are welcome15:42
barryabentley: looking...15:50
barryabentley: is there any way you can redo reply-2 and reply-4 to include the stuff above the comment box?  i'd like to see how that sits with the rest of the page.15:52
abentleybarry: The point is that it jumps to the reply box, and that's all you see on the page.15:53
abentleybarry: The reply box is the regular comment box.15:53
abentleybarry: We just populate it with the in-reply-to comment's text as a quote.15:54
barryabentley: hmm.  in a tall browser window though there'll still be stuff above that, right?  i guess what i can't tell from this is the different between inline and separate page.  (one) point of inline is that you see the context with your reply.  i very much want this because i very much want to see the whole thing when i'm typing my reply15:55
abentleybarry: There's very little difference between inline and separate page, and that's why I've suggested that we should make our existing forms stuff easier to leverage asynchronously.15:56
barryabentley: can you do just one full browser page shot?15:58
abentleybarry: http://people.canonical.com/~abentley/reply-5.png16:12
barryabentley: beautiful!  ui*=me16:12
bacbeuno, mrevell:  why is the button on pop-up help 'Continue' instead of 'Close'?  was that a design decision or did it just happen?16:14
al-maisanhello intellectronica, could you please do me a favour and review a branch of mine? I am off tomorrow and would like to wrap it up before I finish the day.16:14
beunobac, it's a good question. Sounds wrong, but I can't remember any specifics16:15
bacit looks wrong to me16:15
al-maisanintellectronica: just to whet your appetite, it added a method to IBugSet :)16:15
intellectronicaal-maisan: oright, then :)16:15
mrevellbac: I didn't choose the label so I'm not sure.16:16
al-maisanintellectronica: https://code.edge.launchpad.net/~al-maisan/launchpad/clog-oops-452070/+merge/1378916:16
mrevellbac: I can change it.16:16
al-maisanintellectronica: thanks a million!16:16
bacmrevell: that'd be great.16:16
bacbeuno: http://people.canonical.com/~bac/download5.png16:17
bacmrevell: i'm just happy the button shows up now!16:17
beunobac, looks nice. What if we collapsed them together?  "Release notes and changelog"16:17
beunomaybwe probably don't need to much granularity16:18
bacbeuno: hmmm16:18
bacbeuno: i like having them separate.16:18
beunobac, why?16:18
bacbeuno: look what i found yesterday on meetup to introduce new features.  mrevell you might like it to.  http://people.canonical.com/~bac/meetup.png16:19
bacbeuno: so one twisty marked 'Release information' that expands to show release notes and changelog?16:19
mrevellbac: I love it. We discussed something similar at the TL sprint16:19
beunobac, I love that way of showing new features and changes. I may even have an ajax movie showing that on LP  :)16:20
bacnotice that big red "Close" button!16:20
intellectronicaal-maisan: wouldn't it be easier to default preloaded_person_data to {} ? then you won't have to check that it's not None before trying to get the value16:20
bacwith an 'X' too16:20
beunobac, yes. Less clutter on the UI, and may make you make a decision you can't really make16:20
bacbeuno: ok, let me fix that up16:20
beunobac, it's a bit of an open question. I'm interested in why you want them seperate.16:21
al-maisanintellectronica: yeah .. I am always a bit wary of default values like {} or [] because I keep forgetting how the scope rules work.16:21
al-maisanintellectronica: None is clearer and immediately obvious..16:21
bacbeuno: i didn't find it cluttery.  but it's an easy experiment and you make a compelling argument.  let me try it and see16:22
intellectronicaal-maisan: fair enough. the difference is not big anyway16:22
al-maisanintellectronica: yup .. using None saves me a few brain cells a day .. that's all :)16:22
intellectronicaal-maisan: is it really the case that you can only pass strings as the bug numbers to getByBugNubers?16:23
intellectronicaor is it just that storm/postgres coerce strings correctly?16:23
=== salgado is now known as salgado-lunch
al-maisanintellectronica: I did not test for integers .. let me try that.16:24
intellectronicawell, Bug.id is an integer, not a string16:24
intellectronicaif the comparison succeeds it's probably because postgres is not fussy16:24
al-maisanintellectronica: yeah, you are probably right. I'll extend the tests in bug.txt to use normal numbers. That should work as well.16:25
intellectronicaal-maisan: i think it would be enough to change the tests to only use numbers, and remove (strings) from the documentation16:26
al-maisanintellectronica: OK .. fine.16:26
al-maisanby me16:26
intellectronicaal-maisan: is IStore(obj) equivalent to Store.of(obj) ?16:28
al-maisanintellectronica: I understand IStore operates on the class16:28
al-maisanwhereas Store.of() uses objects16:29
intellectronicaoh right16:29
bacbeuno: http://people.canonical.com/~bac/download6.png -- i like16:29
intellectronicabut then, how does it know which store to adapt to?16:29
al-maisanintellectronica: good question .. a (content) class maps to table I guess 16:30
al-maisanintellectronica: http://pastebin.ubuntu.com/299118/16:31
intellectronicaright, but it could, in theory, be the same table in one of several stores16:31
al-maisanintellectronica: master versus on of the slaves? Is that what you are aiming at?16:32
leonardrallenap, since no one is on call maybe you'd be interested in reviewing my branch that fixes bug 451158?16:32
mupBug #451158: AttributeError: 'Credentials' object has no attribute 'authorizeSession' <launchpadlib :New> <https://launchpad.net/bugs/451158>16:32
al-maisanintellectronica: *one of the slaves16:32
* al-maisan looks at the IStore implementation16:33
intellectronicaal-maisan: yeah. just my curiosity, b.t.w, i am not doubting that your code works, i just never saw this form16:33
allenapleonardr: Yeah, okay. I'm in a meeting right now, but after, sure.16:33
al-maisanintellectronica: I used it on a few occasions16:33
beunobac, sold16:33
leonardrallenap, great16:33
intellectronicaal-maisan: r=me. hats off for the impressive improvement!16:34
al-maisanintellectronica: thank you very much!16:35
beunobac, ui=me, and I'm off to lunch16:35
=== beuno is now known as beuno-lunch
=== henninge_ is now known as henninge
=== matsubara is now known as matsubara-lunch
abentleyrockstar: Do you mind doing a UI review of https://code.edge.launchpad.net/~abentley/launchpad/reply/+merge/13739 ?17:07
allenapleonardr: approved :)17:09
rockstarabentley, sure.17:10
=== rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
=== salgado-lunch is now known as salgado
=== EdwinGrubbs is now known as Edwin-lunch
rockstarabentley, r=me17:17
rockstarEr, ui=me17:17
rockstarThat's probably better shorthand.17:17
rockstarabentley, do you know the bug that when you vote with the inline comment, your vote doesn't get inserted into the vote table?17:18
abentleyrockstar: Yes, I just can't bring myself to care.17:18
rockstarabentley, I'll fix it.  I know exactly how to do it.17:19
rockstarabentley, if there's currently a bug open, can you assign it to me?17:19
abentleyrockstar: I don't know if there's a bug open.17:19
rockstarabentley, okay, I'll file one then.17:20
=== beuno-lunch is now known as beuno
=== danilos is now known as danilo-afk
=== matsubara-lunch is now known as matsubara
* rockstar lunches18:55
=== Edwin-lunch is now known as EdwinGrubbs
sidneirockstar: what's the procedure when you get a review with 'needs-fixing'? do you resubmit it with the incremental diff?19:49
sidneiEdwinGrubbs: see my question above?20:15
EdwinGrubbssidnei: You would add a comment to the MP with your replies and also the incremental diff. "Resubmit" normally means creating an entirely new MP, and I assume that's not what you meant.20:17
sidneiEdwinGrubbs: awesome, thanks!20:17
sidneiEdwinGrubbs: i was wondering how the reviewers figure out that its out of 'needs-fixing' and should be reviewed again though.20:18
EdwinGrubbssidnei: I will receive an email with your new comment. You can also notify me directly, if it's urgent.20:19
sidneiEdwinGrubbs: oh. so you're responsible for following up, since you did the first review. makes sense.20:20
jtvrockstar: got two branches in the queue if you're interested in some light reviews20:44
* rockstar returns from lunch20:44
* jtv looks pretty pleased with his timing20:44
rockstarjtv, links to mp?20:46
jtvrockstar: oops, dinner just arrived.  Links follow:20:51
=== salgado is now known as salgado-afk
bachi rockstar -- can you do a review for me?21:25
rockstarbac, absolutely.  Spit it on over.21:25
jtvrockstar: thanks for the review21:38
rockstarjtv, you are welcome.21:38
rockstarbac, have you talked over with beuno why you can't do what he wants?21:39
bacrockstar: not yet.  i've requested a UI review from him even though he approved the previous version21:40
rockstarbac, yea, I was going to suggest that.  I think I'll just perform a code review for now then.21:40
bacrockstar: i didn't realize the additional complication until some of the tests failed and i understood why we can't just whack it21:40
bacrockstar: yes, that would be great21:40
rockstarbac, yeah, that's pretty common.  beuno often finds solutions to satisfy the tests and himself.  He's clever like that.21:41
=== matsubara is now known as matsubara-afk
beunobac, you will want to kill me22:18
beunothat delete button22:18
beunomakes me crazy22:18
bacbeuno: oh, that22:19
beuno"delete files22:19
beunofloating in cyberspace22:19
bacbeuno: yeah, where would you like it?22:19
beunobac, how about we don't show it at all until we click on the checkbox?22:20
bacbeuno: hmmm22:21
beunowe show the button underneath the table22:21
beunodelete individual files and don't tell sinzui22:21
bacbeuno: those sound like good ideas.  but can i convince you to defer them to another branch?  i would like to get this one landed so i can move on to an OOPS that is affecting karmic22:22
beunobac, yes you can, it's not worst than it is now22:23
bacbeuno: no, that button is the same as the bad way i designed it a few years ago22:23
jtvrockstar: still got that other branch on the queue if you feel like doing one more: https://code.edge.launchpad.net/~jtv/launchpad/bug-458049/+merge/1377923:33
rockstarjtv, huh, thought I did that one already23:33
jtvrockstar: you did the other one, for which thanks23:34
rockstarAlso, why the fuck are you awake?23:34
jtvrockstar: look who's talking :-)23:34
rockstarHey man, if you're awake because you're also polyphasic, then I'm okay with that.  If you're awake just because, then man you crazy.23:34
jtvrockstar: no secret that I'm crazy.23:35

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