[06:49] <al-maisan> Good morning 
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
[09:49] <adeuring> al-maisan: could you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/bug-457475-udev-device-id/+merge/13771 ?
[09:50] <al-maisan> adeuring: yep .. will get to it in a few minutes.
[09:50] <adeuring> al-maisan: thanks!
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [abel] || This channel is logged: http://irclogs.ubuntu.com/
[09:54] <adeuring> al-maisan: just noticed that I should add one more tests....
[09:55] <al-maisan> adeuring: that's fine .. please let me know when the branch is ready.
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
[09:55] <adeuring> al-maisan: sure. sorry for the mess....
[09:55] <al-maisan> no problem :)
[10:03] <adeuring> al-maisan: done. See the comment in the MP
[10:05] <al-maisan> adeuring: OK .. thanks.
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
[10:07] <al-maisan> adeuring: your branch is targeting db-devel .. is that right?
[10:08] <adeuring> al-maisan: I'm an idiot... the target should be devel...
[10:08] <al-maisan> adeuring: no worries .. just checking.
[10:18] <al-maisan> adeuring: 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?
=== danilo-afk is now known as danilos
[10:18] <al-maisan> adeuring: how is that device_id for udev device used later on?
[10:18] <al-maisan> *devices
[10:19] <adeuring> al-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 table
[10:19] <al-maisan> adeuring: is the scope of the device_id limited to the respective HWDB report?
[10:19] <adeuring> al-maisan: yes, the ID should be unique only within one submission
[10:20] <al-maisan> adeuring: I see, thanks
[10:22] <adeuring> al-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] <adeuring> s/qt/at/
[10:22] <al-maisan> adeuring: hmm..
[10:26] <al-maisan> adeuring: stupid question re.
[10:26] <al-maisan>         self.assertEqual(1, device.id)
[10:26] <adeuring> al-maisan: yes?
[10:26] <al-maisan> how can you know that the id value is 1
[10:26] <al-maisan> because it is the root/first device?
[10:27] <adeuring> ah, the method setUp() defines self.root_device, and there is the dict item 'id': 1
[10:27] <adeuring> al-maisan: the change of lines 635ff in test_hwdb_submission_parser.py
[10:28] <adeuring> al-maisan: I meant the change in 3036ff in test_hwdb_submission_processing.py
[10:28] <adeuring> (not my best day....)
[10:28] <al-maisan> adeuring: I see .. thanks
[10:28] <al-maisan> adeuring: r=me
[10:28] <adeuring> al-maisan: thanks!
=== 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
[12:47] <adeuring> al-maisan: fancy another review? https://code.edge.launchpad.net/~adeuring/launchpad/bug-458029-kernel-package-name-for-udev-submissions/+merge/13781
[12:49] <al-maisan> adeuring: yes, just give me a few minutes.
[12:49] <adeuring> al-maisan: thanks!
=== 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
[13:46] <al-maisan> adeuring: I need to take a short break and will look at your branch immediately after I return.
[13:47] <adeuring> al-maisan: sure, np
=== matsubara-afk is now known as matsubara
=== gary_poster is now known as gary-sprint
[14:42] <al-maisan> adeuring: looking at your branch now
[14:43] <adeuring> al-maisan: great!
[14:46] <abentley> al-maisan: Could you please review https://code.launchpad.net/~abentley/launchpad/reply/+merge/13739 ?
[14:46] <al-maisan> abentley: as soon as I finish with adeuring's branch
=== 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/
[14:47] <abentley> al-maisan: Thanks!
[14:47] <al-maisan> you are welcome
=== 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/
[14:50] <danilos> al-maisan: hi, I hope you'll have enough time for another review after you are done with adeuring and abentley :)
[14:51] <al-maisan> hello danilos, it may be a bit tight but I should be able to squeeze it in hopefully ;)
[14:59] <danilos> al-maisan: cool, thanks :)
[15:00] <al-maisan> adeuring: r=me
=== 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/
[15:00] <adeuring> al-maisan: thanks!
=== 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/
[15:03] <al-maisan> abentley: is this the MP in question: https://code.launchpad.net/~abentley/lp-production-configs/trunk/+merge/13783 ?
[15:04] <al-maisan> abentley: sorry
[15:04] <al-maisan> just read your qustion above
[15:04] <abentley> al-maisan: no, it's https://code.launchpad.net/~abentley/launchpad/reply/+merge/13739
[15:04] <al-maisan> yup
[15:20] <al-maisan> abentley: r=me
[15:20] <abentley> al-maisan: Thanks!
[15:21] <abentley> barry: Could you do a ui review for me?  https://code.launchpad.net/~abentley/launchpad/reply/+merge/13739
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: - || queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com/
[15:23] <al-maisan> danilos: is this the MP in question: https://code.edge.launchpad.net/~danilo/launchpad/bug-455771/+merge/13699 ?
[15:23] <barry> abentley: sure if you can give me screen shots.  i have no way to run devel branches right now
[15:23] <abentley> al-maisan: Actually, this needs UI review still, so the status should still be work-in-progress.
[15:23] <danilos> al-maisan: no
[15:23] <al-maisan> abentley: OK .. sorry .. will revise it.
[15:24] <abentley> al-maisan: I've already done.
[15:24] <al-maisan> abentley: thanks
[15:25] <danilos> al-maisan: it seems my emails are still not getting through to LP: https://code.edge.launchpad.net/~danilo/launchpad/bug-458036/+merge/13787
[15:25] <al-maisan> danilos: OK
=== al-maisan changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: danilo || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
[15:32] <abentley> barry: Updated with screenshots.
[15:37] <al-maisan> danilos: looks good, r=me
=== 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/
[15:37] <danilos> al-maisan: thanks!
[15:38] <al-maisan> danilos: does your branch need an extra ui review as well?
[15:41] <danilos> al-maisan: I don't think so, I am just changing privileges on the page
[15:41] <danilos> al-maisan: it has been UI reviewed before by beuno
[15:41] <al-maisan> danilos: great .. I'll set it to approved then.
[15:42] <danilos> al-maisan: ta!
[15:42] <al-maisan> you are welcome
[15:50] <barry> abentley: looking...
[15:52] <barry> abentley: 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:53] <abentley> barry: The point is that it jumps to the reply box, and that's all you see on the page.
[15:53] <abentley> barry: The reply box is the regular comment box.
[15:54] <abentley> barry: We just populate it with the in-reply-to comment's text as a quote.
[15:55] <barry> abentley: 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 reply
[15:56] <abentley> barry: 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:58] <barry> abentley: can you do just one full browser page shot?
[16:12] <abentley> barry: http://people.canonical.com/~abentley/reply-5.png
[16:12] <barry> abentley: beautiful!  ui*=me
[16:14] <bac> beuno, 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-maisan> hello 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:15] <beuno> bac, it's a good question. Sounds wrong, but I can't remember any specifics
[16:15] <bac> it looks wrong to me
[16:15] <al-maisan> intellectronica: just to whet your appetite, it added a method to IBugSet :)
[16:15] <al-maisan> *adds
[16:15] <intellectronica> al-maisan: oright, then :)
[16:16] <mrevell> bac: I didn't choose the label so I'm not sure.
[16:16] <al-maisan> intellectronica: https://code.edge.launchpad.net/~al-maisan/launchpad/clog-oops-452070/+merge/13789
[16:16] <mrevell> bac: I can change it.
[16:16] <al-maisan> intellectronica: thanks a million!
[16:16] <bac> mrevell: that'd be great.
[16:17] <bac> beuno: http://people.canonical.com/~bac/download5.png
[16:17] <bac> mrevell: i'm just happy the button shows up now!
[16:17] <mrevell> :)
[16:17] <beuno> bac, looks nice. What if we collapsed them together?  "Release notes and changelog"
[16:18] <beuno> maybwe probably don't need to much granularity
[16:18] <bac> beuno: hmmm
[16:18] <bac> beuno: i like having them separate.
[16:18] <beuno> bac, why?
[16:19] <bac> beuno: look what i found yesterday on meetup to introduce new features.  mrevell you might like it to.  http://people.canonical.com/~bac/meetup.png
[16:19] <bac> beuno: so one twisty marked 'Release information' that expands to show release notes and changelog?
[16:19] <mrevell> bac: I love it. We discussed something similar at the TL sprint
[16:20] <beuno> bac, I love that way of showing new features and changes. I may even have an ajax movie showing that on LP  :)
[16:20] <bac> notice that big red "Close" button!
[16:20] <intellectronica> al-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 value
[16:20] <bac> with an 'X' too
[16:20] <beuno> bac, yes. Less clutter on the UI, and may make you make a decision you can't really make
[16:20] <bac> beuno: ok, let me fix that up
[16:21] <beuno> bac, it's a bit of an open question. I'm interested in why you want them seperate.
[16:21] <al-maisan> intellectronica: yeah .. I am always a bit wary of default values like {} or [] because I keep forgetting how the scope rules work.
[16:21] <al-maisan> intellectronica: None is clearer and immediately obvious..
[16:22] <bac> beuno: i didn't find it cluttery.  but it's an easy experiment and you make a compelling argument.  let me try it and see
[16:22] <intellectronica> al-maisan: fair enough. the difference is not big anyway
[16:22] <al-maisan> intellectronica: yup .. using None saves me a few brain cells a day .. that's all :)
[16:23] <intellectronica> al-maisan: is it really the case that you can only pass strings as the bug numbers to getByBugNubers?
[16:23] <intellectronica> or is it just that storm/postgres coerce strings correctly?
=== salgado is now known as salgado-lunch
[16:24] <al-maisan> intellectronica: I did not test for integers .. let me try that.
[16:24] <intellectronica> well, Bug.id is an integer, not a string
[16:24] <intellectronica> if the comparison succeeds it's probably because postgres is not fussy
[16:25] <al-maisan> intellectronica: yeah, you are probably right. I'll extend the tests in bug.txt to use normal numbers. That should work as well.
[16:26] <intellectronica> al-maisan: i think it would be enough to change the tests to only use numbers, and remove (strings) from the documentation
[16:26] <al-maisan> intellectronica: OK .. fine.
[16:26] <al-maisan> by me
[16:28] <intellectronica> al-maisan: is IStore(obj) equivalent to Store.of(obj) ?
[16:28] <al-maisan> intellectronica: I understand IStore operates on the class
[16:29] <al-maisan> whereas Store.of() uses objects
[16:29] <intellectronica> oh right
[16:29] <bac> beuno: http://people.canonical.com/~bac/download6.png -- i like
[16:29] <intellectronica> but then, how does it know which store to adapt to?
[16:30] <al-maisan> intellectronica: good question .. a (content) class maps to table I guess 
[16:31] <al-maisan> intellectronica: http://pastebin.ubuntu.com/299118/
[16:31] <intellectronica> right, but it could, in theory, be the same table in one of several stores
[16:32] <al-maisan> intellectronica: master versus on of the slaves? Is that what you are aiming at?
[16:32] <leonardr> allenap, since no one is on call maybe you'd be interested in reviewing my branch that fixes bug 451158?
[16:32] <mup> Bug #451158: AttributeError: 'Credentials' object has no attribute 'authorizeSession' <launchpadlib :New> <https://launchpad.net/bugs/451158>
[16:32] <leonardr> https://code.edge.launchpad.net/~leonardr/launchpadlib/oauth-in-restfulclient/+merge/13790
[16:32] <al-maisan> intellectronica: *one of the slaves
[16:33]  * al-maisan looks at the IStore implementation
[16:33] <intellectronica> al-maisan: yeah. just my curiosity, b.t.w, i am not doubting that your code works, i just never saw this form
[16:33] <allenap> leonardr: Yeah, okay. I'm in a meeting right now, but after, sure.
[16:33] <al-maisan> intellectronica: I used it on a few occasions
[16:33] <beuno> bac, sold
[16:33] <leonardr> allenap, great
[16:34] <intellectronica> al-maisan: r=me. hats off for the impressive improvement!
[16:35] <al-maisan> intellectronica: thank you very much!
[16:35] <beuno> bac, ui=me, and I'm off to lunch
=== beuno is now known as beuno-lunch
=== henninge_ is now known as henninge
=== matsubara is now known as matsubara-lunch
[17:07] <abentley> rockstar: Do you mind doing a UI review of https://code.edge.launchpad.net/~abentley/launchpad/reply/+merge/13739 ?
[17:09] <allenap> leonardr: approved :)
[17:10] <rockstar> abentley, sure.
=== 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
[17:17] <rockstar> abentley, r=me
[17:17] <rockstar> Er, ui=me
[17:17] <rockstar> That's probably better shorthand.
[17:18] <rockstar> abentley, 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] <abentley> rockstar: Yes, I just can't bring myself to care.
[17:19] <rockstar> abentley, I'll fix it.  I know exactly how to do it.
[17:19] <rockstar> abentley, if there's currently a bug open, can you assign it to me?
[17:19] <abentley> rockstar: I don't know if there's a bug open.
[17:20] <rockstar> abentley, okay, I'll file one then.
=== beuno-lunch is now known as beuno
=== danilos is now known as danilo-afk
=== matsubara-lunch is now known as matsubara
[18:55]  * rockstar lunches
=== Edwin-lunch is now known as EdwinGrubbs
[19:49] <sidnei> rockstar: what's the procedure when you get a review with 'needs-fixing'? do you resubmit it with the incremental diff?
[20:15] <sidnei> EdwinGrubbs: see my question above?
[20:15] <sidnei> ^^
[20:17] <EdwinGrubbs> sidnei: 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] <sidnei> EdwinGrubbs: awesome, thanks!
[20:18] <sidnei> EdwinGrubbs: i was wondering how the reviewers figure out that its out of 'needs-fixing' and should be reviewed again though.
[20:19] <EdwinGrubbs> sidnei: I will receive an email with your new comment. You can also notify me directly, if it's urgent.
[20:20] <sidnei> EdwinGrubbs: oh. so you're responsible for following up, since you did the first review. makes sense.
[20:44] <jtv> rockstar: got two branches in the queue if you're interested in some light reviews
[20:44]  * rockstar returns from lunch
[20:44]  * jtv looks pretty pleased with his timing
[20:46] <rockstar> jtv, links to mp?
[20:51] <jtv> rockstar: oops, dinner just arrived.  Links follow:
[20:51] <jtv> https://code.edge.launchpad.net/~jtv/launchpad/bug-457987/+merge/13774
[20:51] <jtv> https://code.edge.launchpad.net/~jtv/launchpad/bug-458049/+merge/13779
=== salgado is now known as salgado-afk
[21:25] <bac> hi rockstar -- can you do a review for me?
[21:25] <rockstar> bac, absolutely.  Spit it on over.
[21:25] <bac> https://code.edge.launchpad.net/~bac/launchpad/bug-419742/+merge/13807
[21:38] <jtv> rockstar: thanks for the review
[21:38] <rockstar> jtv, you are welcome.
[21:39] <rockstar> bac, have you talked over with beuno why you can't do what he wants?
[21:40] <bac> rockstar: not yet.  i've requested a UI review from him even though he approved the previous version
[21:40] <rockstar> bac, yea, I was going to suggest that.  I think I'll just perform a code review for now then.
[21:40] <bac> rockstar: i didn't realize the additional complication until some of the tests failed and i understood why we can't just whack it
[21:40] <bac> rockstar: yes, that would be great
[21:41] <rockstar> bac, yeah, that's pretty common.  beuno often finds solutions to satisfy the tests and himself.  He's clever like that.
=== matsubara is now known as matsubara-afk
[22:18] <beuno> bac, you will want to kill me
[22:18] <beuno> but
[22:18] <beuno> that delete button
[22:18] <beuno> makes me crazy
[22:19] <bac> ?
[22:19] <bac> beuno: oh, that
[22:19] <beuno> "delete files
[22:19] <beuno> floating in cyberspace
[22:19] <bac> beuno: yeah, where would you like it?
[22:20] <beuno> bac, how about we don't show it at all until we click on the checkbox?
[22:21] <beuno> and
[22:21] <bac> beuno: hmmm
[22:21] <beuno> we show the button underneath the table
[22:21] <beuno> or
[22:21] <beuno> delete individual files and don't tell sinzui
[22:22] <bac> beuno: 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 karmic
[22:23] <beuno> bac, yes you can, it's not worst than it is now
[22:23] <bac> beuno: no, that button is the same as the bad way i designed it a few years ago
[22:23] <beuno> :)
[23:33] <jtv> rockstar: 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/13779
[23:33] <rockstar> jtv, huh, thought I did that one already
[23:34] <jtv> rockstar: you did the other one, for which thanks
[23:34] <rockstar> Also, why the fuck are you awake?
[23:34] <jtv> rockstar: look who's talking :-)
[23:34] <rockstar> Hey 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:35] <jtv> rockstar: no secret that I'm crazy.