Code review comment for ~dirk.zimoch/epics-base:CleanupWhitespace

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> 'git blame -w' to ignore whitespace change?

To my mind this is the important point. I'm fairly open about accepting the occasional reformatting which 'git diff --ignore-all-space' renders as empty. And I would be fairly closed to reformatting which doesn't. So eg. no moving brackets between lines. Also, this isn't something to be done frequently. (once per decade is ok)

This change is almost empty. 'git diff --ignore-all-space' still shows the removal of blank lines at the end of files. I'm not so bother this time, although I don't see much point to doing this in future.

As far as style rules. I'm not so strict on aspects other than indentation. There my first rule is to be consistent. With indentation of 4 spaces being a very strong preference (where allowed).

Beyond that, I'm not so concerned beyond a small bias towards schemes which qtcreator can apply automatically. In short, this is not an area where I want to spend much time.

So this change is acceptable to me, provided it doesn't complicate upmerge by whatever version of Git Andrew has these days.

I'm marking this "Needs Fixing" since Dirk has indicated he has more work to do.

review: Needs Fixing

« Back to merge proposal