Best Practices with code/build fixes post-merge?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Best Practices with code/build fixes post-merge?

Robert Dailey
Sometimes, I merge 2 branches that have deviated quite a bit. A
worst-case example would be some API change. The topic branch
(long-lived) may start using the old API. However, once I merge the
topic back to master, that API no longer exists. As such, every place
that introduces a usage of the old API will fail to build (but won't
necessarily cause a conflict during a merge).

Concerning best practices, which of the following is better?

1. Make the fixes (which may be vast), smoke test, get a general feel
that everything is working on master again. Amend the changes to the
previous merge commit.

2. Make the fixes as in step 1, but instead of amending to the merge
commit, create a new descendant commit representing the changes.

Concerns I see with either choice:

1. Pros: Changes are atomic. Cons: merge commits are typically very
difficult to view, especially from logs. For example, `git log
--name-status` does nothing for merge commits, so it makes it
less-intuitive to get a good overview of changes.

2. Opposite of #1: Changes are not atomic (con), but it makes it very
clear that these changes were made "on top of" the merge (pro)

Is there a best practice here? What do each of you do as a personal
preference or policy? Thanks in advance.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: Best Practices with code/build fixes post-merge?

Junio C Hamano
Robert Dailey <[hidden email]> writes:

> Sometimes, I merge 2 branches that have deviated quite a bit. A
> worst-case example would be some API change. The topic branch
> (long-lived) may start using the old API. However, once I merge the
> topic back to master, that API no longer exists. As such, every place
> that introduces a usage of the old API will fail to build (but won't
> necessarily cause a conflict during a merge).
> ...
> Is there a best practice here? What do each of you do as a personal
> preference or policy? Thanks in advance.

I think the best practice is to make sure your "some API change" is
done in a way competent people do.  Namely, have a thin wrapper to
emulate the old API so that such a long-lived topic that adds new
caller of old API would still be syntactically and semantically
correct after getting textually merged.

Your developers however are not perfect (nobody is), so the next
best is to arrange that a build fails (which seems to be what you
have), so that you can notice at the merge time.

If the adjustment of an application that uses the old API to use the
new one is simple enough, amending the merge would be my preference,
as a merge commit should be "a single atomic change" as any other
commit.  But at the same time, you do not have to be too worried
about such a "mismerge".  A merge commit is a change that can
introduce a bug as any other commit.  After all, we are dealing with
human endeavours.  So "I did this merge as a single atomic change,
merging the work done in the topic while adjusting the way the topic
did certain things using old API to use new API, to the best of my
ability, but I may have introduced a bug" is perfectly fine.  You do
a commit to ensure it is correct to the best of your ability, and
because everybody knows you are not perfect (nobody is), nobody
expects your commit is always correct.  If you find a bug later, you
just fix up with a follow-up commit.  A merge commit is no special.

My personal preference, if adjustment from old API to new API is
more involved, is to have a new topic branch that is forked from the
trunk and merge the topic with new call sites of old API into it.
I'd fix as much breakage in that merge commit, and then fix things
up as necessary in follow up commits on that new topic branch as
necessary.  After that, merging the result to the trunk.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html