classification
Title: mention issues with code churn in the devguide
Type: Stage:
Components: Devguide Versions:
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: eric.araujo, ezio.melotti, loewis, tshepang, willingc
Priority: normal Keywords:

Created on 2013-05-23 03:37 by tshepang, last changed 2016-07-22 19:55 by brett.cannon. This issue is now closed.

Messages (6)
msg189844 - (view) Author: Tshepang Lekhonkhobe (tshepang) * Date: 2013-05-23 03:37
I've seen complaints about code churn a few times, example: http://bugs.python.org/issue16510#msg175956. Would be nice if this was documented in the devguide.
msg189904 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2013-05-24 09:57
What does "churn" mean?
msg189920 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-05-24 16:43
Moving things around.
msg189921 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-05-24 16:46
Tshepang is referring to cases where a patch changes one line but affects ten other lines because a paragraph was rewrapped, modernizing code, updating tests to use latest helpers, etc.

I’ve actually seen two contrary kinds of advice: do cleanups when you touch the code for a bug fix or new feature (the holistic approach / anti-code churn) vs. separate concerns with separate commits.
msg190818 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-06-08 19:33
> I’ve actually seen two contrary kinds of advice

and the best approach is most of the times something in between.  Doing minor cleanups while touching the code is fine, but if most of the changes are cleanups then it gets distracting.  In that case it's better to do the cleanups in a separate commit (either before or after).

Regarding rewrapping paragraphs in the docs the situation is a bit different.  There are 3 options:
1) change and rewrap in the same commit;
2) change but avoid rewrapping so that the changes are easier to detect;
3) like 2, but then add the rewrapping in a separate commit;

3) is overkill IMHO, and I don't think I've seen it used very often.
2) makes the diff more readable, but leaves the code unwrapped making it less readable for everyone else (and I would argue that reading code happens more often than reading diffs).
1) makes the diff less readable, but leaves wrapped code and it's IMHO the best compromise, and thus what I usually do.


However, I think that the actual problem pointed out in the linked issue is about mass changes, rather than cleanups while touching code for other reasons.  Even for that it depends on the case: how many files/lines are affected, what benefits are brought by the change, how complex are the changes, how likely is to introduce new bugs while doing them and so on.

A couple of generic guidelines could be added to the devguide, but there's no hard rule for this.
msg241089 - (view) Author: Carol Willing (willingc) * (Python committer) Date: 2015-04-15 08:42
https://bugs.python.org/issue23320 provides more recent input on reflow of paragraphs. This newer issue should be consulted when creating a patch for this issue. Ideally, both issues should be able to be closed with the same patch.
History
Date User Action Args
2016-07-22 19:55:20brett.cannonsetstatus: open -> closed
resolution: out of date
2015-04-15 08:42:22willingcsetnosy: + willingc
messages: + msg241089
2013-06-08 19:33:25ezio.melottisetmessages: + msg190818
2013-05-24 16:46:19eric.araujosetmessages: + msg189921
2013-05-24 16:43:58eric.araujosetnosy: + eric.araujo
messages: + msg189920
2013-05-24 09:57:04loewissetnosy: + loewis
messages: + msg189904
2013-05-23 03:37:31tshepangcreate