=== mrevell is now known as mrevell-lunch
=== mrevell-lunch is now known as mrevell
MootBotMeeting started at 09:00. The chair is bac.15:00
MootBotCommands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]15:00
bacwelcome back flacoste!15:01
bac[topic] * Agenda15:01
bac * Outstanding actions15:01
bac * New topics15:01
bac  * canonical.launchpad is still deprecated, sinzui15:01
MootBotNew Topic:  * Agenda15:01
bac  * branch size metric15:01
bac * Mentat update.15:01
bac * Peanut gallery15:01
bacEdwinGrubbs: ping15:02
EdwinGrubbsme, again15:02
bacEdwinGrubbs, sorry15:02
bac[topic] Outstanding actions15:03
MootBotNew Topic:  Outstanding actions15:03
bac* bac and lifeless to chat about branch size.15:03
bacdone.  we'll discuss it later15:03
bacnothing else outstanding.  \o/15:03
bac[topic] * canonical.launchpad is still deprecated, sinzui15:04
MootBotNew Topic:  * canonical.launchpad is still deprecated, sinzui15:04
baccan you elaborate, sinzui?15:04
sinzuiReviewer, please challenge developer to move code out of c.l instead of making changes in it15:04
sinzuiNo developer should be adding new code to it15:05
sinzuic.l is in a bad state at the moment...15:05
sinzuino one claims ownership of the code in it...it needs owners15:05
sinzuic.l contributes to our cyclic import problem. you will not be happy until c.l is gone15:06
henningeAlso, elimiate any import from canonical.launchpad.interfaces when you see one!15:06
marssinzui, +1.  Also, it is easy to split the branch to keep the size down if you use bzr pipes15:06
* sinzui has a branch that fixes all zcml15:07
henningemostly in doctests, I think15:07
henningeoh yeah, and zcml15:07
sinzuimars, You probably need to split the branch15:07
sinzuiI think most of the work now involves moving classes, not modules15:07
=== Ursinha-afk is now known as Ursinha
sinzuiBut look at interfaces and database. Some team must own it, otherwise, lets talk about deleting it15:08
* sinzui eyes account15:08
bacthanks sinzui15:09
wgrantWhy aren't c.l.i and c.l.d imports lint?15:09
sinzuino one has written an extension to report that.15:09
sinzuiIt might be easier to get people to commit to just fixing all the import over the next two weeks15:10
sinzuiI have nothing more to say15:11
bacsinzui: why don't y'all discuss that at the TL meeting and see if the teams can schedule the work?15:11
noodles775I think teams other than code/translations need to add lp/app/interfaces/webservice.py before we can remove c.l.i?15:11
wgrantThat's true.15:12
wgrantAnd rather unobvious.15:12
sinzuiI have had four failed branches to remove c.l.i15:12
sinzuisoyuz and registry are in a death spiral15:12
wgrantsinzui: Circular imports galore?15:12
sinzuiwgrant, yes15:13
bac[topic] * branch size metric15:13
MootBotNew Topic:  * branch size metric15:13
sinzuiI am now trying to remove chunks by theme like lp.<model> or all zcml15:13
baca few weeks ago in the asiapac meeting we started discussing the metric we use for branch sizes.  currently we use 800 lines of diff.15:14
baclifeless made the argument that this is a "bad surrogate" for managing branch focus and complexity, which is what we really want to control15:14
bacso is the 800 line rule still useful and valid?  we used to have severe problems with branches being too large but as a team we seem to have trained ourselves out of that bad habit15:16
danilosI find the rule is still working fine as a checkpoint, and we should keep it15:17
sinzuiI think 2000 lines of mechanical changes made by a find and replace is easier to review than a branch that changes things for 5 very different reasons15:17
marsbac, imho yes, the rule is a valid rule of thumb.  It still applies broadly, and for some operations, we all know when it can be bent15:17
marssinzui, exactly15:17
abentleysinzui: I think it's a good guideline, but sensible people know when to ignore it.15:17
bacsinzui: that is basically the argument.  focus is more important than size15:17
flacostei agree with mars15:17
sinzuiI think 4 hours is a good checkpoint15:17
deryckis there something someone wants to do that the 800 line limit prevents?  Or does lifeless see a better way to manage scope?15:18
* bac hides15:18
abentleysinzui: you mean, after 4 hours of work, we should submit something for review?15:18
marsbac, also, there is research that shows that code reviews over 400 lines get difficult.  I treat 400 as ideal, 800 as a hard limit for green-field code.  the 800 line diff rule fits into a "400 lines of change plus context"15:18
sinzuiabentley, maybe. If the branch has something that is valuable and landable15:19
baclifeless's argument is that diff size is a silly measure.  800 lines of deletes is very different than an 500 line contiguous addition which is different from a bunch of scattered one line changes.15:19
deryckbac, but is there an alternate proposal for what is a good measure?  or just a request to drop the requirement?15:19
abentleybac: my argument is that there's nothing that's reasonably easy to check that won't be silly.15:19
bacthere was an experimental one deryck15:20
danilos800 limit is simple, above all... we shouldn't be spending too much time wondering if can get our branches reviewed; 800 line limit is for us to stop and think if we go above it15:20
marsbac, true, but we all know the difference, don't we?  The reviewee always says "This is huge, but it's deletes, so please don't lynch me"15:20
bacin order to control focus, robert suggested a branch that takes more than 2-3 major discussion points to describe (in the MP for example) is probably too unfocused15:20
danilos(i.e. it's just a signal that we might be doing something wrong: i.e. you are solving several things with one branch, etc.)15:20
gmbbac: But not everyone writes a good MP :)15:20
bacgmb: don't i know it!  :)15:21
deryckyeah, I agree with gmb, that only indicates language facility, not coding focus :-)15:21
gmbLine count at least gives us reviewers something to fall back to and say "Er, that's a bit on the large side..."15:21
allenapThe 800 line limit makes me think about the focus of the branch, and puts a lid on things in case they do get a bit unfocused.15:21
danilosbac, sometimes I describe the background for a branch in more than a few points, to assist translations-unaware reviewer :)15:21
sinzuiThe implementation meeting should be setting scope and what issues a branch needs to solve15:21
bacdanilos: but those aren't describing the core changes15:21
* sinzui often has a separate branch for drive-by fixes15:22
marsbac, so would we like to see, say, instead of a 600-line 2 topic branch, two 300-line one-topic dependent branches?15:22
bacmars: ideally, yes15:23
marsIf so, may I suggest the 400 line soft limit again?  more than 800 lines is a burden on the reivewer, but more than 400 means you may have to split topics15:23
bacmars: well that is how this discussion got started15:23
bacmars: rockstar brought up the idea of dropping the limit to 40015:24
mars~400 lines will speed the review and is likely to keep it topical15:24
deryckbac, so is the concern that 800 doesn't really limit scope, or that 800 is too constricting for some work?15:24
bacpersonally he found it more manageable and more productive15:24
marsbigjools, hehe, I have research from IBM to prove it :)15:24
marsit was C++, mind you15:24
danilosI, honestly, don't see how 400 is any better than 800... except that it will be hard to live with in some cases (I had new tests take around 400 lines for some very intricate methods)15:25
bacderyck: the argument is the 800 limit is wobbly, often ignored, and never was a good indicator and that rules have cost.  a bad rule should be removed or replaced with a good rule;15:25
deryckso by that, any line limit is a bad rule, right?15:26
* bigjools doesn't see why this has to be so bureaucratic15:26
bacthis topic is not pressing, but i wanted us to talk about it15:26
baci think it is better discussed in person, so i'll put it on the schedule for dallas15:26
danilos800-line-limit rule is cheap compared to any other, and I agree we should not be spending too much of our time on discussing it15:26
marsbac, it sounds like many people liked the 800-line rule as a guideline.  Do people feel it was ignored?15:26
deryckI think there needs to be a really great counter proposal to make this a useful discussion.15:26
danilosderyck, +115:27
marsderyck, yes15:27
henningemars: I don't.15:27
henningederyck: yes, that's what's missing here.15:27
marshenninge, you don't like it?  Or you don't follow it? :)15:27
bac[action] move discussion of branch size/complexity to the epic --bac15:27
MootBotACTION received:  move discussion of branch size/complexity to the epic --bac15:27
henningemars: ;-) I don't feel it was ignored.15:27
henningebac: +115:28
henningebut only if there is a good aleternative to discuss.15:28
bac[topic] mentoring update15:28
MootBotNew Topic:  mentoring update15:28
danilos"It's going to be an Epic discussion!"15:29
* bac forgets who here is being mentored15:29
bacsalgado, how is it going?15:29
* henninge is UI mentat15:29
daniloshenninge, salgado as UI reviewers15:29
salgadobac, I'm enjoying it!15:29
bachenninge: getting any UI reviews?15:29
salgadoand getting plenty of requests15:29
henningebac: yes, I have15:30
bacsalgado is the anti-beuno when it comes to screen real estate15:30
henningebeen getting some.15:30
* sinzui has an 800px wide browser like most lp users15:30
bacso, as a reminder, throw your UI reviews to those to first if you can15:31
bacand save up some code reviews for stevenk if possible15:31
bac[topic] anything else?15:31
MootBotNew Topic:  anything else?15:31
abentleybac, yes: line breaks.15:31
sinzuihttp://browsersize.googlelabs.com/ <- https://launchpad.net15:31
MootBotLINK received:  http://browsersize.googlelabs.com/ <- https://launchpad.net15:31
abentleyI thought the rules for where we should put newlines were clear, and specified in PEP8, but it seems like that really only covers method definitions.15:32
abentleyFor example, I thought we were supposed to have a blank line between class variable declarations, but I can't find a rule for that.15:32
abentleyDo we have rules about this?  Should we?15:33
bacabentley: if you didn't find it in https://dev.launchpad.net/PythonStyleGuide then i guess we do not15:34
bacabentley: have you seen lots of inconsistency?15:35
abentleyAre there any rules that people have been following that we should codify?15:35
henningeI have not been putting new lines between class attributes consistently.15:36
henningeIn interfaces, yes, but in test cases, no.15:37
abentleyOkay, I guess I'll just ignore that in reviews.15:37
* henninge has not changed an interface in a long time ...15:37
bacabentley: thanks for bringing it up but it doesn't seem to be a huge problem people are seeing.15:38
bacany other topic?15:39
MootBotMeeting finished at 09:39.15:39
henningethanks bac! ;)15:39
bacthanks all15:39
marsthanks bac15:39
=== Ursinha is now known as Ursinha-lunch
=== salgado is now known as salgado-lunch
=== Ursinha-lunch is now known as Ursinha
=== salgado-lunch is now known as salgado
=== salgado is now known as salgado-brb
=== salgado-brb is now known as salgado
=== Ursinha is now known as Ursinha-bbl
=== salgado is now known as salgado-afk

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