On 24/05/2013 13:00, Johan Hake wrote:
...
On 24 May 2013 13:35, Lawrence Mitchell <[email protected]
<mailto:[email protected]>> wrote:
...
3. When ready submit as pull request.
4. Respond to review comments by making changes in my branch.
Where appropriate, these would either be as new commits, or
fixups squashed into existing commits (perhaps fixing a typo). Like
a good git citizen, I rerolled my patches in response to review
comments.
Why is it bad git habit to push small typo commits? Especially when it
potentially will screw the history compared to an already published and
pushed branch on the main dolfin repository?
As Garth says, there are mixed views. One argument in favour of
rerolling over pushing new commits on top is that you would like every
commit that is merged into master to be buildable and passing the test
suite. That way bisection to find regressions can be entirely automated.
Imagine the following scenario:
Developer commits:
#ifdef SOME_FEATURE
do_stuff();
#else
do_other_stuff()
#endif
But only ever test-compiled with SOME_FEATURE enabled, and therefore
didn't notice that the do_other_stuff line was missing a semi colon.
Now, this will be spotted in review and they have two choices.
1. Fix up the error in a separate commit
2. Fix the error and squash it into the original commit (so that it
looks like it never happpened).
In the former case we might have something like:
A---B---C---D---E---fixup
^ ^
error is here fix is here
In the latter:
A'---B'---C'---D'---E'
^
no error here
Now if the first case is merged, an automatic bisection to find a bug
might spuriously stop at A (because the build doesn't work), needing
manual intervention to fix. In the latter, that wouldn't happen.
Cheers,
Lawrence
--
The University of Edinburgh is a charitable body, registered in
Scotland, with registration number SC005336.
_______________________________________________
fenics mailing list
[email protected]
http://fenicsproject.org/mailman/listinfo/fenics
