cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joerg Heinicke <joerg.heini...@gmx.de>
Subject Re: [POLL] don't edit files just for style changes?
Date Sat, 17 Apr 2004 15:31:29 GMT
On 17.04.2004 16:18, Bertrand Delacretaz wrote:

> Lately there have been several hints that people are annoyed by commits 
> consisting only of style changes (rearranging imports, "cleaning up" 
> indents) etc.
> 
> Of course, there are always good intentions behind such changes, but 
> IMHO changes which make no difference to the actual code, are:
> 
> a) risky, because every time one edits a file there's a small risk of 
> making a change to the code without meaning it
> 
> b) a waste of our collective energy, as (hopefully) many of us are 
> regularly scanning CVS diff mails, and if it's just to find out that two 
> brackets have been moved it's not very useful.
> 
> I have three suggestions:
> 
> 1) As a general rule, we should refrain from making such changes to our 
> code, unless there is a good reason (code change for example) to edit 
> the file in question.

Code quality is IMO a good reason. Therefore I have set my eclipse 
compiler probably a bit more receptive than others, e.g. imports, unused 
local variables and private fields, javadocs. But for using those 
features the problems list must also stay at a manageable extent. This 
is the only reason I touch many of the files I would not even have 
looked on without the compiler warnings.

> 2) When editing a file for a good reason, we should look for such 
> "style" problems and fix them at the same time, or as a separate commit, 
> but before testing our changes.

If the style problem comprises not only a few lines it's important to 
commit it separately IMO. Otherwise the actual code change gets lost.

> 3) We should not change the indentation / code writing style (brackets 
> etc.) of a file when making a minor change to it.

+1

> The idea is to keep the flow of CVS diff messages as low as possible, to 
> save our neuronal bandwidth for more important things than the placement 
> of brackets or the order of imports.
> 
> WDYT?

IMO it's only important to separate code changes clearly from style 
changes. For an example see 
http://thread.gmane.org/gmane.text.xml.cocoon.cvs/11901. A replacement 
of hand-written code with commons lang code got completely lost in a 
huge list of style changes. It was really only luck that I saw the 
faulty change at one place when flying over the diff.

Joerg

Mime
View raw message