| === mrevell is now known as mrevell-lunch | ||
| === mrevell-lunch is now known as mrevell | ||
| bac | #startmeeting | 15:00 |
|---|---|---|
| MootBot | Meeting started at 09:00. The chair is bac. | 15:00 |
| MootBot | Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] | 15:00 |
| flacoste | me | 15:00 |
| noodles775 | me | 15:00 |
| henninge | me | 15:00 |
| EdwinGrubbs | me | 15:00 |
| allenap | me | 15:00 |
| sinzui | me | 15:00 |
| adeuring | me | 15:00 |
| gmb | me | 15:00 |
| danilos | mi | 15:00 |
| jelmer | me | 15:00 |
| abentley | me | 15:00 |
| bac | welcome back flacoste! | 15:01 |
| flacoste | thanks | 15:01 |
| bac | [topic] * Agenda | 15:01 |
| bac | * Outstanding actions | 15:01 |
| bac | * New topics | 15:01 |
| bac | * canonical.launchpad is still deprecated, sinzui | 15:01 |
| MootBot | New Topic: * Agenda | 15:01 |
| bac | * branch size metric | 15:01 |
| bac | * Mentat update. | 15:01 |
| bac | * Peanut gallery | 15:01 |
| bac | EdwinGrubbs: ping | 15:02 |
| deryck | me | 15:02 |
| EdwinGrubbs | me, again | 15:02 |
| bac | EdwinGrubbs, sorry | 15:02 |
| bac | :( | 15:02 |
| mars | me | 15:03 |
| leonardr | m | 15:03 |
| leonardr | e | 15:03 |
| bac | [topic] Outstanding actions | 15:03 |
| MootBot | New Topic: Outstanding actions | 15:03 |
| bac | * bac and lifeless to chat about branch size. | 15:03 |
| bac | done. we'll discuss it later | 15:03 |
| bac | nothing else outstanding. \o/ | 15:03 |
| bac | [topic] * canonical.launchpad is still deprecated, sinzui | 15:04 |
| MootBot | New Topic: * canonical.launchpad is still deprecated, sinzui | 15:04 |
| bac | can you elaborate, sinzui? | 15:04 |
| sinzui | Reviewer, please challenge developer to move code out of c.l instead of making changes in it | 15:04 |
| sinzui | No developer should be adding new code to it | 15:05 |
| sinzui | c.l is in a bad state at the moment... | 15:05 |
| sinzui | no one claims ownership of the code in it...it needs owners | 15:05 |
| sinzui | c.l contributes to our cyclic import problem. you will not be happy until c.l is gone | 15:06 |
| henninge | Also, elimiate any import from canonical.launchpad.interfaces when you see one! | 15:06 |
| mars | sinzui, +1. Also, it is easy to split the branch to keep the size down if you use bzr pipes | 15:06 |
| * sinzui has a branch that fixes all zcml | 15:07 | |
| henninge | mostly in doctests, I think | 15:07 |
| henninge | oh yeah, and zcml | 15:07 |
| sinzui | mars, You probably need to split the branch | 15:07 |
| sinzui | I think most of the work now involves moving classes, not modules | 15:07 |
| === Ursinha-afk is now known as Ursinha | ||
| sinzui | But look at interfaces and database. Some team must own it, otherwise, lets talk about deleting it | 15:08 |
| * sinzui eyes account | 15:08 | |
| bac | thanks sinzui | 15:09 |
| wgrant | Why aren't c.l.i and c.l.d imports lint? | 15:09 |
| sinzui | no one has written an extension to report that. | 15:09 |
| sinzui | It might be easier to get people to commit to just fixing all the import over the next two weeks | 15:10 |
| sinzui | I have nothing more to say | 15:11 |
| bac | sinzui: why don't y'all discuss that at the TL meeting and see if the teams can schedule the work? | 15:11 |
| noodles775 | I think teams other than code/translations need to add lp/app/interfaces/webservice.py before we can remove c.l.i? | 15:11 |
| wgrant | That's true. | 15:12 |
| wgrant | And rather unobvious. | 15:12 |
| sinzui | I have had four failed branches to remove c.l.i | 15:12 |
| sinzui | soyuz and registry are in a death spiral | 15:12 |
| wgrant | sinzui: Circular imports galore? | 15:12 |
| sinzui | wgrant, yes | 15:13 |
| wgrant | :( | 15:13 |
| bac | [topic] * branch size metric | 15:13 |
| MootBot | New Topic: * branch size metric | 15:13 |
| sinzui | I am now trying to remove chunks by theme like lp.<model> or all zcml | 15:13 |
| bac | a 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 |
| bac | lifeless made the argument that this is a "bad surrogate" for managing branch focus and complexity, which is what we really want to control | 15:14 |
| bac | so 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 habit | 15:16 |
| danilos | I find the rule is still working fine as a checkpoint, and we should keep it | 15:17 |
| sinzui | I 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 reasons | 15:17 |
| mars | bac, 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 bent | 15:17 |
| mars | sinzui, exactly | 15:17 |
| abentley | sinzui: I think it's a good guideline, but sensible people know when to ignore it. | 15:17 |
| bac | sinzui: that is basically the argument. focus is more important than size | 15:17 |
| flacoste | i agree with mars | 15:17 |
| sinzui | I think 4 hours is a good checkpoint | 15:17 |
| deryck | is 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 hides | 15:18 | |
| abentley | sinzui: you mean, after 4 hours of work, we should submit something for review? | 15:18 |
| mars | bac, 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 |
| sinzui | abentley, maybe. If the branch has something that is valuable and landable | 15:19 |
| bac | lifeless'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 |
| deryck | bac, but is there an alternate proposal for what is a good measure? or just a request to drop the requirement? | 15:19 |
| abentley | bac: my argument is that there's nothing that's reasonably easy to check that won't be silly. | 15:19 |
| bac | there was an experimental one deryck | 15:20 |
| danilos | 800 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 it | 15:20 |
| mars | bac, 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 |
| bac | in 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 unfocused | 15: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 |
| gmb | bac: But not everyone writes a good MP :) | 15:20 |
| bac | gmb: don't i know it! :) | 15:21 |
| deryck | yeah, I agree with gmb, that only indicates language facility, not coding focus :-) | 15:21 |
| gmb | Line count at least gives us reviewers something to fall back to and say "Er, that's a bit on the large side..." | 15:21 |
| allenap | The 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 |
| danilos | bac, sometimes I describe the background for a branch in more than a few points, to assist translations-unaware reviewer :) | 15:21 |
| sinzui | The implementation meeting should be setting scope and what issues a branch needs to solve | 15:21 |
| bac | danilos: but those aren't describing the core changes | 15:21 |
| * sinzui often has a separate branch for drive-by fixes | 15:22 | |
| mars | bac, so would we like to see, say, instead of a 600-line 2 topic branch, two 300-line one-topic dependent branches? | 15:22 |
| bac | mars: ideally, yes | 15:23 |
| mars | If 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 topics | 15:23 |
| bac | mars: well that is how this discussion got started | 15:23 |
| bac | mars: rockstar brought up the idea of dropping the limit to 400 | 15:24 |
| mars | ~400 lines will speed the review and is likely to keep it topical | 15:24 |
| bigjools | mauve.bikeshed.com | 15:24 |
| deryck | bac, so is the concern that 800 doesn't really limit scope, or that 800 is too constricting for some work? | 15:24 |
| bac | personally he found it more manageable and more productive | 15:24 |
| mars | bigjools, hehe, I have research from IBM to prove it :) | 15:24 |
| mars | it was C++, mind you | 15:24 |
| danilos | I, 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 |
| bac | deryck: 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 |
| deryck | ok | 15:26 |
| deryck | so by that, any line limit is a bad rule, right? | 15:26 |
| * bigjools doesn't see why this has to be so bureaucratic | 15:26 | |
| bac | this topic is not pressing, but i wanted us to talk about it | 15:26 |
| bac | i think it is better discussed in person, so i'll put it on the schedule for dallas | 15:26 |
| danilos | 800-line-limit rule is cheap compared to any other, and I agree we should not be spending too much of our time on discussing it | 15:26 |
| mars | bac, it sounds like many people liked the 800-line rule as a guideline. Do people feel it was ignored? | 15:26 |
| deryck | I think there needs to be a really great counter proposal to make this a useful discussion. | 15:26 |
| danilos | deryck, +1 | 15:27 |
| mars | deryck, yes | 15:27 |
| henninge | mars: I don't. | 15:27 |
| henninge | deryck: yes, that's what's missing here. | 15:27 |
| mars | henninge, you don't like it? Or you don't follow it? :) | 15:27 |
| bac | [action] move discussion of branch size/complexity to the epic --bac | 15:27 |
| MootBot | ACTION received: move discussion of branch size/complexity to the epic --bac | 15:27 |
| henninge | mars: ;-) I don't feel it was ignored. | 15:27 |
| henninge | bac: +1 | 15:28 |
| henninge | but only if there is a good aleternative to discuss. | 15:28 |
| bac | [topic] mentoring update | 15:28 |
| MootBot | New Topic: mentoring update | 15:28 |
| danilos | "It's going to be an Epic discussion!" | 15:29 |
| * bac forgets who here is being mentored | 15:29 | |
| bac | salgado, how is it going? | 15:29 |
| * henninge is UI mentat | 15:29 | |
| danilos | henninge, salgado as UI reviewers | 15:29 |
| salgado | bac, I'm enjoying it! | 15:29 |
| bac | henninge: getting any UI reviews? | 15:29 |
| salgado | and getting plenty of requests | 15:29 |
| bac | great | 15:29 |
| henninge | bac: yes, I have | 15:30 |
| bac | salgado is the anti-beuno when it comes to screen real estate | 15:30 |
| henninge | been getting some. | 15:30 |
| * sinzui has an 800px wide browser like most lp users | 15:30 | |
| bac | so, as a reminder, throw your UI reviews to those to first if you can | 15:31 |
| bac | and save up some code reviews for stevenk if possible | 15:31 |
| bac | [topic] anything else? | 15:31 |
| MootBot | New Topic: anything else? | 15:31 |
| abentley | bac, yes: line breaks. | 15:31 |
| sinzui | http://browsersize.googlelabs.com/ <- https://launchpad.net | 15:31 |
| MootBot | LINK received: http://browsersize.googlelabs.com/ <- https://launchpad.net | 15:31 |
| abentley | I 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 |
| abentley | For 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 |
| abentley | Do we have rules about this? Should we? | 15:33 |
| bac | abentley: if you didn't find it in https://dev.launchpad.net/PythonStyleGuide then i guess we do not | 15:34 |
| bac | abentley: have you seen lots of inconsistency? | 15:35 |
| abentley | Are there any rules that people have been following that we should codify? | 15:35 |
| henninge | I have not been putting new lines between class attributes consistently. | 15:36 |
| henninge | In interfaces, yes, but in test cases, no. | 15:37 |
| abentley | Okay, I guess I'll just ignore that in reviews. | 15:37 |
| * henninge has not changed an interface in a long time ... | 15:37 | |
| bac | abentley: thanks for bringing it up but it doesn't seem to be a huge problem people are seeing. | 15:38 |
| bac | any other topic? | 15:39 |
| bac | 3 | 15:39 |
| bac | 2 | 15:39 |
| bac | 1 | 15:39 |
| bac | #endmeeting | 15:39 |
| MootBot | Meeting finished at 09:39. | 15:39 |
| henninge | thanks bac! ;) | 15:39 |
| bac | thanks all | 15:39 |
| mars | thanks bac | 15: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!