>> 3. How to best split up patches from that category? e.g. fixing all
>> "if(...){" -> "if (...) {" occurrences in qtbase would be a rather
>> large patch (I have it half done)
>> a) split that up based on "manageable" subdirectories?
>>
> yes, and that only for easy recovery from merge conflicts (which you
> will get quite some of in highly active areas). other than that, make it
> one huge whitespace cleanup change: remove trailing whitespace, expand
> tabs (which for fun have inconsistent expectations regarding their
> size), fix indentation, fix mixplaced line breaks (the brace re-wrapping
> falls into this category), add missing spaces, etc.
I thought fixing everything at once everywhere is obviously a bad idea:
* merge conflicts until the end of time
* too much burden to come up with and also to review the patch
Especially the combination is problematic: a huge patch will only be
looked at when someone is 'really bored', because it takes such an
effort, the longer the patch lays around, the more likely a merge
conflict.
Hence, splitting it up is inevitable. I thought of two split up strategies:
a) split up by the 'type' of style problem (like the above mentioned change)
b) split up by file/directory and try to fix every 'type' at once
I thought splitting by type first and potentially by directory second
if it is too big still, would be best because it reduces the burden on
the reviewer: only one type of change should be easier to skim over
and might actually be checked by some ad-hock script if that was
necessary. Or does the automatic check for white-space only changes
make it secure enough so it can basically be waved through without
having to look at each hunk?
> somebody attempted this just a few days ago, but gave up when the scope
> of the undertaking became clear. i can't find the change now ...
I don't think the requirement to only accept a patch if it fixes
'every' whitespace problem in one go would be reasonable as it would
result in exactly this situation: too much work to do/review it all at
once so one gives up.
Therefore I propose to accept fixes, even if they don't fix every
issue (either in type of file space) at once: better improve something
step by step, than nothing at all.
>> b) only do it in files where it is used inconsistently / where the
>> wrong occurrences are the minority?
>>
> no, do it consistently - to have a good example everywhere, and no
> excuses for messing up. then we can make the automatic checking
> stricter, too (which lars said was a precondition for even bothering
> with a cleanup to start with).
As a final goal, it should obviously be consistent everywhere but I
thought it makes sense to make a difference between files where there
are some random style issues here and there (most of the code) and
files that are written pretty consistently but according to a
different style guide (like the qmake generator stuff). The latter
might probably be best fixed in an 'all types at once but on a
file/directory basis' approach, so each patch would increase the
'local' consistency and be an improvement even if I or someone else
would loose interest at some point in their white space cleaning
spree.
To summarize my suggestion:
* accept partly style fixes (in either type or file space)
* accept them on an a timely manner to make it feasible at all with
regards to merge conflicts
* intentionally ignore stuff in src/3rdparty
I might even be interested in looking into the automatic style check
issue, but can't promise anything.
> have fun ^^
Thanks ;) Depending on your feedback, I'll give it a try. ;)
- Axel
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
