/srv/irclogs.ubuntu.com/2008/12/10/#launchpad-meeting.txt

=== salgado-afk is now known as salgado
=== mrevell is now known as mrevell-lunch
=== mrevell-lunch is now known as mrevell
barry#startmeeting15:00
MootBotMeeting started at 09:00. The chair is barry.15:00
MootBotCommands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]15:00
barryhello and welcome to this week's ameu reviewers meeting.  who's here today?15:00
rockstarme15:00
EdwinGrubbsme15:00
adeuringme15:00
flacosteme15:01
rockstarThat's it?15:02
barrywow, light attendance today <wink>15:02
barryallenap: ping15:02
allenapme15:02
barrybac_uds: hopeful ping15:02
barrybigjools: ping15:02
barryBjornT: ping15:02
barrycprov: ping15:02
bigjoolsme15:02
barrygmb: ping?15:02
barryintellectronica: ping15:03
barrymars: ping15:03
barrysalgado: ping15:03
bigjoolsit's 7 in the morning for cprov at UDS15:03
intellectronicame15:03
BjornTme15:03
barrybigjools: what's his cell #?  let's wake him up :)15:03
* barry is not sure who's at uds15:03
rockstarOoh, that's bad...15:03
bigjools1-800-EAT-SHIT15:03
rockstar!15:03
* barry dials15:03
marsme15:03
barrybigjools: hey, waiiittt a minute!15:04
bigjools:)15:04
barry[TOPIC] agenda15:04
MootBotNew Topic:  agenda15:04
barry * Roll call15:04
barry * Printing strings in doctests (barry)15:04
barry * Gavin's `pretty()` function (allenap)15:04
barry * Do we need a standard cover letter template for merge proposals? (barry)15:04
barry   * [[attachment:cover-quick.txt]]15:04
barry   * [[attachment:cover.txt]]15:04
barry * Peanut gallery (anything not on the agenda)15:04
barry * If there's time, the old boring script15:04
barry   * Next meeting15:04
barry   * Action items15:04
barry   * Mentoring update15:04
barry   * Queue status15:04
barry[TOPIC]  * Printing strings in doctests (barry)15:04
MootBotNew Topic:   * Printing strings in doctests (barry)15:04
barryso, i mentioned this in a couple of reviews as well as in the asiapac meeting, with some positive feedback15:05
barrythe idea is that in docstrings, you should (usually) be printing the value of strings instead of returning them15:05
sinzuime15:05
barrythe advantage of that is that you usually don't care if its unicode or str, and you're not saddled with extraneous noise like u' and '15:05
barrye.g.15:05
barryinstead of15:05
barry>>> foo['key']15:05
barryu'baz'15:05
barryyou'd do15:06
barry>>> print foo['key']15:06
barrybaz15:06
barrythoughts?15:06
salgadome!15:06
salgadosorry, I'm late15:06
intellectronicayes, i also prefer printing15:06
bigjoolsbarry: +115:06
sinzuiUnless we fix/hack the the testrunner, translations and answers tests will fail15:07
barrysinzui: right.  hence the "usually" :)15:07
rockstarbarry, +115:07
barryso, as reviewers just be aware of that in code you review and suggest conversion to printing strings15:08
barryany objections?15:08
adeuringnone, just a questin:15:08
barryadeuring: go4it15:09
EdwinGrubbsI think that the bug really needs to be fixed in the testrunner, since it doesn't tell you which line the error is on when you try printing out a unicode character, the whole doctest just fails, so you end up commenting out half your test  to find the offending line.15:09
adeuringwhat, if we want to test, if we want to check that we have for eaxmpale a string object, but not a unicode object?15:09
adeuringprint repr(foo)15:09
rockstarEdwinGrubbs, +115:09
barryEdwinGrubbs: totally agree.  it's not like we don't actually have proposed fixes either :)  iirc it's a bug in zope upstream15:09
adeuringI mean, whould we use explicity the print statment,15:10
barryadeuring: in that case you can either return the value and show the u''15:10
adeuringor should we allow the simple >>> foo15:10
barryadeuring: or, use isinstance(foo, unicode)15:10
barryadeuring: or even print foo.encode('utf-8')15:10
rockstarI'd prefer the second choice.15:10
barryrockstar: for a pure type test, i agree15:11
rockstarbarry, yea' and if you want the value, you'd use print.15:11
adeuringbarry: OK, though explicit encoding can make things more difficult to read15:11
barryrockstar: right15:11
barryadeuring: yep.  if we had a fixed testrunner we could just print it15:12
barryflacoste: can we ask gary to look into fixing this upstream for us?15:12
adeuringbarry: ...or use repr(u'äüö')15:12
flacostebarry: gary doesn't have on the testrunner15:12
adeuringslightly better to read than the encoded string15:12
flacostebarry: but he can send emails15:12
flacostebarry: but anyway, we are way behind upstream here15:12
flacostebarry: so we should just fix our stuff for now15:13
rockstarflacoste, +115:13
barryflacoste: maybe when we move to 2.6 :)15:13
barryand zc.buildout :)15:13
flacoste2.6 is crack!15:13
barry:-D15:13
barryanyway for now, just be aware of this in reviews and make friendly suggestions15:13
barry[TOPIC]  * Gavin's `pretty()` function (allenap)15:14
MootBotNew Topic:   * Gavin's `pretty()` function (allenap)15:14
barryallenap landed a branch with a nice pretty() function in doctest globals15:14
flacostedon't use it for API tests btw!15:14
barryalthough i've since heard that lazr has something similar, right flacoste ?15:14
sinzuizope.testing.doctest:148615:14
sinzui    sys.stdout = self._fakeout15:14
sinzuiBecomes15:14
sinzui    from codecs import EncodedFile15:14
sinzui    sys.stdout = EncodedFile(self._fakeout, "ascii", "utf-8")15:14
allenapflacoste: too late.15:14
barryflacoste: why?15:14
barrysinzui: yes, excactly15:14
flacostebecause there is pprint_entry and pprint_collection15:14
flacostethat does that + other stuff15:15
barryflacoste: ah15:15
flacostefor example, it omits the etag key15:15
flacosteso pretty() is fine for non-API stuff15:15
barryflacoste: do we need a dev/reviewer style guide for apis?15:15
flacosteyeah, we probably do15:15
allenapflacoste: Yeah, agreed, the pprint_* functions are better for API. I'll clean up my API stuff that uses pretty().15:15
flacostesalgado started something if i recall15:15
flacosteor maybe it was for the coder perspective15:16
flacostebut yes, i'll do a API reviewer cheatsheet15:16
barry[ACTION] flacoste to do an API reviewer cheatsheet15:16
MootBotACTION received:  flacoste to do an API reviewer cheatsheet15:16
barryflacoste: thanks!15:16
flacosteor add a section to the reviewer cheet sheet15:16
barrybtw, rs=barry for any cleanup that consolidates pretty() and lazr's pretty printer15:17
salgadoflacoste, barry, https://launchpad.canonical.com/API/StyleGuide is what I wrote15:17
barrysalgado: nice, thanks15:18
barry[TOPIC]  * Do we need a standard cover letter template for merge proposals? (barry)15:18
MootBotNew Topic:   * Do we need a standard cover letter template for merge proposals? (barry)15:18
barryi mentioned this last week.  this week i have two examples.  one a quick outline and the other more detailed, with examples15:18
barryhttps://dev.launchpad.net/ReviewerMeetingAgenda?action=AttachFile&do=view&target=cover-quick.txt15:19
barryhttps://dev.launchpad.net/ReviewerMeetingAgenda?action=AttachFile&do=view&target=cover.txt15:19
barryputting aside the issue of tool support for this, what do you think about encouraging devs to use this in mps?15:19
barrythe thing i like about it is that everything's in one comment15:19
barryso when you get the mp email, you've already got the description, the lint, and the diff15:20
barryand it makes it very easy to respond in email15:20
barryi really hate it when i have to review a branch, but it's spread out over several emails15:20
rockstarbarry, I don't like that either.15:21
rockstarI usually try and consolidate it into one reply.15:21
barryrockstar: right, which isn't always easy depending on your mua :)15:21
rockstarYea.15:22
rockstar(vim ftw)15:22
barryrockstar: claws + emacs + emacsclient :)15:22
bigjoolsjeez, copy and paste ffs :)15:22
barryanyway.  +1, -1  for the cover standard?15:22
bigjools+015:22
rockstarbarry, which one is would be the standard?15:23
barryrockstar: they're the same except for the explanatory text and examples in the long one15:23
barryrockstar: so cover-quick.txt is the template most people would use15:23
bigjoolshow much of the stuff in the template should we / can we put in the MP form?15:24
* sinzui has been pasting it all15:24
barrysinzui: right.  the idea is that you begin filling out the form right when you start working on the branch.  lots will be empty, but you fill it in as you think about the fix, get a pre-imp, hack the code, etc15:25
barrywhen you're ready for the mp, you paste the whole thing15:25
rockstarbarry, why do we need bug XXXXXX section?15:25
barryrockstar: as the longer example says, it's to translate the bug description into human15:25
sinzuibarry: correct. I do,15:26
barryfrom the native "user" :)15:26
rockstarHm.  I think if I read the bug itself, and then read the solution, I could deduce the issues myself.15:26
barryrockstar: remember, we're trying to make things fast and easy for reviewers15:27
bigjoolsdoesn't the branch link to the bug if you say --fixes?15:27
barryrockstar: so the dev should do more work upfront (imo)15:27
rockstarbigjools, yes15:27
=== salgado is now known as salgado-lunch
barryrockstar: the idea is.  i'm going to review your branch in 5 minutes.  i should have everything i need right there in that one email15:27
rockstarbarry, I'm all for people making the reviewer's life easier.15:27
barryrockstar: don't make me go looking around the world unless i want to15:27
bigjoolsI think that doing that and having the reviewer follow a link is better than the dev re-writing the bug description15:27
barrybigjools: the problem is that many bug have poor descriptions, or long conversational threads15:28
barrybigjools: i think it's the dev's resposibility to boil it down to: "Bug XYZ describes this very specific problem"15:28
rockstarbarry, so why can't that translation to human be in the bug.15:28
bigjoolsbarry: so I would rather the root cause was fixed then - lets update the bug15:28
barryi guess the point is that i don't want to have to hunt around in the bug.  i want to read my email, do my review and be done with it15:29
rockstarbarry, maybe that would make sense as a sentence in the pre-implementation of implementation section.15:29
bigjoolsI hear we've got this funky tool called Launchpad that stores all this stuff for us ;)15:30
barryi should try to find a real-world example to show you what i mean.  it's really just a 2 sentence summary15:30
barryand you can just cut-n-paste the bug description if it's in human readable form15:30
rockstarIf there's good information about the bug itself, I feel it really belongs in the bug, not the merge proposal.15:30
barrybut if it's not, or the bug has a long conversational thread that eventually gets around to the real bug, then 2 sentences, boom you're done, and you've helped reduce reviewer friction15:31
rockstarIt may be just two lines, but it's still segmentation of information.15:31
bigjoolslet me propose this:  if the bug's description was included in the branch page (is it already?) and the email, and the description was updated if it's inaccurate, would that suffice?15:31
barryrockstar: this isn't a big detailed thing, it's a summary.  the cover can always say there are important details in the tracker issue15:31
rockstarQA might not know to go look at the merge proposal for the actual explanation.15:32
sinzuirockstar: I summarize the bug because I don't want to make you spend 20 minutes reading all the comments that were not relevant to diagnosing the root cause15:32
barrybigjools: sure.  if it were included in the email that would help15:32
barrysinzui: exactly15:32
rockstarsinzui, I understand that, and I agree it's helpful.  I just think that we have all these buckets where certain types of information go.15:33
bigjoolsbarry: great - I just hate the idea of re-writing the bug description and not including it in the bug report!15:33
barrythis is all about reducing reviewer friction15:33
rockstarbarry, and I agree it needs to happen.15:33
rockstarI also like to make sure that the branch is linked to the appropriate bugs it fixes.15:33
sinzuirockstar: secondly, I may land five branches to fix a bug, each branch focuses on an aspect of the bug that is strictly scoped so that I have small branches15:33
barrybigjools: i have no problem if the workflow is: dev updates the bug description to accurately reflect the root cause of the problem being reported, then cut-and-pastes that into his cover letter :)15:33
BjornTi think we should rename 'Bug XXX' to 'Summary', and concentrate on describing what the branch contains, rather than what the bug was.15:33
barrysinzui: excellent point!15:34
rockstarBjornT, +115:34
intellectronica+1, and we already have that in the submission form, so the only thing is to make sure that it's displayed conveniently both on the web and in email15:34
bigjoolsBjornT: again, I think that should be recorded in LP, with the branch.  It can be sent with the email.15:34
barryBjornT: Summary of Bug XXXXX and i'm on board with that15:34
bigjoolsbarry: not all branches are bug fixes though15:35
barrybigjools: hmm. okay15:35
BjornTbarry: well, as sinzui said, he might land five branches to fx15:35
barrybut they should be, right? :)15:35
BjornTfix a single bug15:35
bigjoolsbarry: blueprints? :)15:35
rockstarbarry, two lines in of the faur line Summary can be devoted to it.15:35
BjornTbarry: should he include the same bug description in all five merge proposals?15:35
rockstars/faur/four15:35
barrybigjools: i've heard it's a great way to increase your karma15:35
bigjoolsbarry: you get more for blueprints! :)15:36
barryBjornT: when i've done that, the first sentence is the same, but then the rest of that summary section goes on to explain the sub-issue this branch is addressing15:36
barrybut i'm okay with Summary15:36
BjornTbarry: right. in that case you're not summarizing the bug report; you're summarizing your branch15:37
barryas long as i don't have to guess which bug it's addressing15:37
barryBjornT: cool, i'm convinced.  Summary sounds good15:37
barryi'll make that change and take it to the ml15:38
BjornTi do agree that the summary should explain in words what issue was fixed, rather than just saying 'fixes bug xxx'.15:38
barrybtw, i really appreciate all the good feedback.  ultimately this is about making things easier for us reviewers!15:38
barryyou should work harder when your dev hat is on :)15:38
barryBjornT: +115:39
bigjoolsas someone once said to me, we're being worked like camels already :)15:39
barrybigjools: :)  reducing dev friction is a whole 'nuther topic15:39
barrybut not for today :)15:40
bigjools5 mins left!15:40
barry[TOPIC] # Peanut gallery (anything not on the agenda)15:40
MootBotNew Topic:  # Peanut gallery (anything not on the agenda)15:40
barrybigjools: what a great straight man you are!15:40
barryso, the floor is open.  do you have anything not on the agenda?15:40
rockstarHow are merge proposals going?15:41
rockstarWe're fixing bugs to get things easier to use.15:41
bigjoolsI think they're great, but I think they could prompt us for the info that barry wants :)15:41
rockstarIs anyone noticing?15:41
flacostebigjools: honestly, i don't think so15:41
flacosteform filling is boring15:42
barryrockstar: you had me at "PendingReviews is dead".  i think they're great15:42
bigjoolsflacoste: you'd rather copy & paste a template?15:42
sinzuiflacoste: but note taking is safe and satisfying.15:42
rockstarbarry, Hurray!15:42
flacosteyeah, free form text FTW!15:42
barrytool support would definitely make it easier15:42
barryrockstar: plus i want a button to re-send me the mp email :)15:43
bigjoolsI like being reminded of things, I'm not getting any younger!15:43
BjornTflacoste: +115:43
barryflacoste, rockstar how much of the mp stuff is exposed in the api?15:44
BjornTbigjools: you should be able to use 'bzr send' and have the template come up in your text editor.15:44
bigjoolsBjornT: bzr send creates MPs?15:44
rockstarbarry, the basic read only stuff landed last week.15:44
barryBjornT: though of course, you've already started the template when you began to think about the bug, had your pre-impl, etc. <wink>15:44
BjornTbigjools: well, somehow you should be able to submit MPs from bzr15:44
rockstarbigjools, it will, as soon as abentley gets back from UDS.15:44
barryBjornT: big +1 there15:45
rockstarBjornT, yea, that's the idea.15:45
bigjoolsBjornT: agree15:45
bigjoolsrockstar: that would be very cool!15:45
intellectronicapersonally, i would rather have web forms (and perhaps the ability to bypass them using the API)15:45
intellectronicaand i think they would be very good for are users too15:45
bigjoolswell both - choice is good15:45
rockstarSo diffs and bzr send are the big ones for mp right now then?15:45
barryrockstar: i think so15:46
barryoops, look at the clock.  we're the time go?15:46
rockstarCool.  I'll bring that up in the meeting.  I know the code has been landing, just don't know the specifics.15:46
barryanything else before we break?15:46
barry515:46
BjornTrockstar: especially the possiblity to attach diffs (rather than having them generated automatically, which is also nice of course)15:46
barry415:47
barry315:47
rockstarBjornT, agreed15:47
barry215:47
barry115:47
rockstarBreak!15:47
barry#endmeeting15:47
MootBotMeeting finished at 09:47.15:47
barrythanks everyone!15:47
allenapThanks barry.15:47
rockstarBjornT, technically, bzr send uses attachments to send the diff.  Is that what you're saying?15:48
BjornTrockstar: kind of. what i want is to have the diff attached to the outgoing e-mail notification (not appended as normal text). this is for both new merge proposals, and for replies, where the coder is attaching a diff of his changes.15:49
rockstarAh, so when you receive a mp email, the diff is an attachment?15:50
BjornTrockstar: yes, that is what i want. that way it's easy to get to the diff. it's easier to go to the next attachment, rather than paging through the comment to find the beginning of the diff.15:51
rockstarBjornT, okay, noted.  I'll see what I can do.15:51
BjornTrockstar: cool, thanks.15:52
=== salgado-lunch is now known as salgado
=== salgado is now known as salgado-brb
=== salgado-brb is now known as salgado
=== salgado is now known as salgado-afk

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