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

Revision history for this message
Andrew Johnson (anj) wrote :

I am concerned that merging this as is will make it difficult to merge commits made on the 3.15 branch up into the 7.0 branch. I just spent quite a few hours doing just that, without having to contend with a large number of lines that had white-space changes as well as code a documentation changes. Git's recursive merge strategy supports the flag -Xignore-space-change which ought to help, but for several of the conflicts I just had to fix git didn't make any attempt to do a 3-way merge at all, because there were file renames involved as well as large content changes, and it didn't recognize these as such.

If you don't think this will be a problem, please try checking out this branch and doing a git merge of the 3.15 branch into it. I got 63 lines of output with 31 CONFLICT lines when I did that last night to the 7.0 branch.

Don't get me wrong I would like us to have a unified source format and coding style (which of course would follow *my* code-formatting preferences, n'est-pas?), but since we have 2 branches to support I don't think this is the best way to achieve that (nor would doing this in the 3.15 branch and merging up be much better). This approach is fine for the PVA submodules because they don't have any older branches to support though.

We might be able to minimize the churn if we add a .indent.pro file and reformat both branches identically, then merge up with no code changes, but I'd want to try that out before committing to the approach. Are GNU indent and BSD indent (as provided on MacOS) fully compatible? I would also only do this once we have minimized the number of outstanding merge & pull requests.

« Back to merge proposal