How to commit incomplete changes?

How to commit incomplete changes?

[Hallvard B Furuseth]

 Do people have any feelings or conventions for how and when to publish
 a series of commits where the first one(s) break something and the next
 ones clear it up?  I've found some discussion, but with vague results.

 I'm about to commit some small edits which go together with bigger
 generated changes.  It seems both more readable and more cherry-pick-
 friendly to me to keep these in separate commits.

 What I've found is I can use a line in the commit message like
     "Incomplete change, requires next commit (update foo/ dir)."
 and, if there is any point, do a no-ff merge past the breakage.

-- 
 Hallvard

Re: How to commit incomplete changes?

[Alexey Shumkin]

>  Do people have any feelings or conventions for how and when to
> publish a series of commits where the first one(s) break something
> and the next ones clear it up?
I'm curiuos, why to you want to commit changes that break something
separately from fixup?

Many conventions (as I know) use ideology that every commit must NOT
BREAK existing code or tests. Every SHARED commit. Git design (as you
must be already know) allows you to make/change/reorder as many commits
as you want (before you share them or push to a "central" repository).
So, you have not to be afraid to commit every your change, because you
can always change/fixup/split your commits.

Usually, you introduce a feature in a branch. Also, your project must
have (?) (mine do have) unit-tests, at least. And most changes must be
tested. So, breakage must be discovered early, even after some other
commits in that feature branch. In that case you can just make a fixup
commit and then rebase it on a breakage commit with a "squash".

And only after all features made and all tests passed you can share
them (push to another repo).

>I've found some discussion, but with vague results.
> 
>  I'm about to commit some small edits which go together with bigger
>  generated changes.  It seems both more readable and more cherry-pick-
>  friendly to me to keep these in separate commits.
> 
>  What I've found is I can use a line in the commit message like
>      "Incomplete change, requires next commit (update foo/ dir)."
>  and, if there is any point, do a no-ff merge past the breakage.
> 

Re: How to commit incomplete changes?

[Hallvard B Furuseth]

 On Thu, 15 Dec 2011 10:44:44 +0400, Alexey Shumkin 
 <Alex.Crezoff@gmail.com> wrote:
>> Do people have any feelings or conventions for how and when to
>> publish a series of commits where the first one(s) break something
>> and the next ones clear it up?
>
> I'm curiuos, why to you want to commit changes that break something
> separately from fixup?

 I answered that, but maybe too briefly:

>>  I'm about to commit some small edits which go together with bigger
>>  generated changes.  It seems both more readable and more 
>> cherry-pick-
>>  friendly to me to keep these in separate commits.

 To expand on that: To review the change, review the hand-edited 
 commits,
 which is easier when these do not drown in generated changes.  Review
 the *commands* which generated the rest - I'd put those in the commit
 message - and glance at the actual changes.  Cherry-pick: Possbly you
 need to run the commands instead of cherry-picking the generated
 changes.  That's easier with a commit with only generated changes.

 I know it also can cause problems.  Would you make a single big commit
 anyway, and describe carefully in the commit message which parts are
 hand-edits?  (We don't auto-test commits yet, but I'll sure this issue
 will crop up again later when we do.)

-- 
 Hallvard

Re: How to commit incomplete changes?

[Alexey Shumkin]

Oh! I got it. I missed "generated changes".
Well, unfortunately (or fortunately ;) ), I did not meet such a workflow
when changes are "generated" without my hands.
In your case it may sound reasonable to make separate fixup commit.
But Git allows you to make your own (more flexible than SVN,
for instance) workflow, which suits you. It's up to you ;)
You decide.
If you plan cherry-picking that fixups, do separate fixups. Just
publish them together.
If you want every commit is "clear" and "workable", squash fixup into
a single commit.
I do not know what exactly is "generated changes" you're talking
about ), so, maybe I'd do separate fixups, maybe not. ))
There is no single solution. ))) TIMTOWTDI
That is why you hesitate :) Do your own decision. And feel free to
change it later. ))

>  To expand on that: To review the change, review the hand-edited 
>  commits,
>  which is easier when these do not drown in generated changes.  Review
>  the *commands* which generated the rest - I'd put those in the commit
>  message - and glance at the actual changes.  Cherry-pick: Possbly you
>  need to run the commands instead of cherry-picking the generated
>  changes.  That's easier with a commit with only generated changes.
> 
>  I know it also can cause problems.  Would you make a single big
> commit anyway, and describe carefully in the commit message which
> parts are hand-edits?  (We don't auto-test commits yet, but I'll sure
> this issue will crop up again later when we do.)
> 

Re: How to commit incomplete changes?

[Alexey Shumkin]

> Do your own decision. 
* "Make your own decision", of course :)

Re: How to commit incomplete changes?

[Neal Kreitzinger]

On 12/14/2011 5:24 PM, Hallvard B Furuseth wrote:
> Do people have any feelings or conventions for how and when to publish
> a series of commits where the first one(s) break something and the next
> ones clear it up? I've found some discussion, but with vague results.
>
> I'm about to commit some small edits which go together with bigger
> generated changes. It seems both more readable and more cherry-pick-
> friendly to me to keep these in separate commits.
>
> What I've found is I can use a line in the commit message like
> "Incomplete change, requires next commit (update foo/ dir)."
> and, if there is any point, do a no-ff merge past the breakage.
>
A main purpose for the squash and fixup options is (as Randall Schwartz 
put it in his git video http://www.youtube.com/watch?v=8dhZ9BXQgc4) "To 
make it look like you did it all perfectly without making any mistakes" 
(or a reasonable facsimile thereof).  You insights on the cherry-picking 
of fixes is interesting, but makes no sense in the context of 
unpublished work.  Why would you need to cherry-pick fixes to mistakes 
that have not yet been propagated (published)?  If the cherry-picks of 
fixes are for your other already merged local branches then just save 
the pre-squash/fixup version of the branch to another branch, (ie, git 
branch mybranch-b4-fixup) and cherry-pick from that unsquashed copy to 
patch up your other unpublished branches.  Keep in mind that cherry-pick 
is not alway the best way to apply fixes.  A merge or rebase to get the 
fix is the sign of a better workflow in many cases, TBOMK. On the other 
hand, if the bugs have been published then you have no choice but to 
commit the fix separately because you can't rearrage/edit published 
history.  Keep in mind that ideally commits should be logical.  You can 
use the rearrage feature of interactive rebase to squash fixes into the 
feature commit they go to. IOW, I don't think squashing everything into 
a giant commit just to consolidate bugfixes into a single commit makes 
sense if that would mean losing the distinct separation between 
differing feature commits.

I assume by 'generated changes' you mean the automerge in git that is a 
wonderful default for vast systems like the linux kernel in which code 
is unlikely to overlap logically, but very dangerous in legacy 
application systems where changes to the same file can create logical 
bugs despite not being on the 'exact same line of code'.  You are 
supposed to review all your merged files after a merge regardless. 
However, we don't trust ourselves that much in our shop so we force 
conflicts on same-file edits by making "user-date stamp" updates on 
"line 1" (depends on language-dependent comment line rules) in our 
pre-commit hook.  That way we are forced to manually review the merge of 
same-file edits "by hand" thus avoiding "generated results".  Of course, 
unique-file edits can still break things and thus a merge review is 
still in order.

Hope this helps.  I'm not a git workflow expert, but my comments are 
based on experience.  I too am still looking for better ways to manage 
workflow while leveraging the flexibity and agility of git for 
concurrent development.

v/r,
neal

Re: How to commit incomplete changes?

[Junio C Hamano]

Neal Kreitzinger <nkreitzinger@gmail.com> writes:

> A main purpose for the squash and fixup options is ...
> "To make it look like you
> did it all perfectly without making any mistakes" (or a reasonable
> facsimile thereof).  You insights on the cherry-picking of fixes is
> interesting, but makes no sense in the context of unpublished work.
> Why would you need to cherry-pick fixes to mistakes that have not yet
> been propagated (published)?
> ...
> I assume by 'generated changes' you mean the automerge in git...

My reading of the "need to split" example was not "bulk of work plus fixes
to mistakes". Imagine you are working on somebody else's code and for some
reason you want to do

	s/setenv/xsetenv/g

all over the code, and also add a wrapper to implement xsetenv() function.

You _could_ do it in one single commit, but what happens when you try to
adjust to the updated upstream code, which may have added new callsites to
setenv()?

If you keep it as two patches, one is mechanical (i.e. s/setenv/xsetenv/g)
and the other is manual (i.e. implementation of xsetenv()), then you can
discard the text of the "mechanical" one from the old series and instead
run the substitution on the updated code, and then cherry-pick the
"manual" one.

If you did the mechanical one first, the resulting code would not compile
(lacks xsetenv() implementation), and then the second "manual" one would
"fix" it. In this simplified example, it is easy to flip the orders and
keep things work, but then you would get a complaint from clever compiler
or linker that xsetenv() implementation is defined but nobody uses it,
which is another kind of breakage. So it _is_ possible that you cannot
avoid breaking the system inside two patches, making them "all-or-none"
series.

Re: How to commit incomplete changes?

[Tomas Carnecky]

On 12/15/11 12:24 AM, Hallvard B Furuseth wrote:
> I'm about to commit some small edits which go together with bigger
> generated changes.  It seems both more readable and more cherry-pick-
> friendly to me to keep these in separate commits.
Why do you store generated code? Usually you only store what is 
absolutely necessary. That means generated code should be generated as 
needed (part of the build process for example).

tom

Re: How to commit incomplete changes?

[Hallvard Breien Furuseth]

[Neal Kreitzinger]
> I assume by 'generated changes' you mean the automerge in git...

No.  And to your questions of why I want this with unpublished work:
No.  Like I wrote, I'm talking about published commits.

[Junio C Hamano]
> My reading of the "need to split" example was not "bulk of work plus fixes
> to mistakes".  Imagine you are working on somebody else's code and for some
> reason you want to do
> 
> 	s/setenv/xsetenv/g
> 
> all over the code, and also add a wrapper to implement xsetenv() function.

Yes - except there is no "mistakes" since it's deliberate.  I'd do
s/setenv/xsetenv/g, which does too little (misses some preprocessor
stuff) or is too greedy, then commit anyway and clean up in next commit.

I could make and commit a much more complicated script to do it all, but
that's unhelpful when trying to read what the heck the change is doing.
And who knows what it'd do when run on a somewhat different codebase.

That example matches a future internal API change.  My current issue
is changes generated with 'autoreconf' - after cleaning up an utter
libtool/automake mess by hand, which will break things if I don't
autoreconf in the same commmit.

> You _could_ do it in one single commit, but what happens when you try to
> adjust to the updated upstream code, which may have added new callsites to
> setenv()?

Indeed.  In this case, it'd be when cherry-picking from the devel branch
to the release branch.  These still differ too much, a legacy of our old
CVS workflow.

> If you keep it as two patches, one is mechanical (i.e. s/setenv/xsetenv/g)
> and the other is manual (i.e. implementation of xsetenv()), then you can
> discard the text of the "mechanical" one from the old series and instead
> run the substitution on the updated code, and then cherry-pick the
> "manual" one.

Yes.  I'd order it in a sequence which never broke anything if I could.

Well, come to think of it: Possibly I could introduce some new code
which would only exist for the sake of patching over the temporary
breakage, and then delete that code again 2-3 commits later.  In this
case, I'd among other things create an obsolete libtool.m4 which is
currently hiding inside aclocal.m4.  Not sure if that makes more sense
than just having a few broken commits.

-- 
Hallvard

Re: How to commit incomplete changes?

[Hallvard Breien Furuseth]

I wrote:
>[Neal Kreitzinger]
>> I assume by 'generated changes' you mean the automerge in git...
> 
> No.  And to your questions of why I want this with unpublished work:
> No.  Like I wrote, I'm talking about published commits.

Wait, I see - as-yet unpublished commits, yes.  Which seem best to me to
publish that way.  And which will get cherry-picked later, after commit
and testing.

-- 
Hallvard