On Thu, Sep 11, 2008 at 05:39:45PM +0200, Thomas Rast wrote:
Thank you for your attempt. It is clearly a missing part of the Git
documentation. I have a few comments to it below.
I would rather add some explanation why it is a good idea. Something
like this:
"This makes the review process much easier, as well as, makes git bisect
much more useful in finding the cause of regressions."
I like the idea of this paragraph but not its wording. Maybe this will
be better (just a variant):
"To achieve this, try to split your work in small steps from the very
beginning. It is always easier to squash a few commits together than
splitting one big commit into a few. Don't be afraid making steps too
small or that they are not perfect yet. You can always go back later and
edit the commits with `git rebase --interactive` before you publish it."
Perhaps, the above two points are the most controversial in my opinion.
First, I would expect developers to implement a few iterations until
it (their work) passes the automated test suite and peer review. Only
then their work is merged into 'next' (or into a similar branch, which
constitute that this series is published now).
Second, I am not sure what you meant by testers clears changes for
stable releases, especially after you stated "Testers" may be an
automated test suite. Whether some change is included is always a
conscious decision of the project maintainer. The fact that some change
has passed all tests successfully only clears it for including into
'next'.
The idea of 'next' is not obvious from your above explanation. When I
started to learn how Git workflow works, I read something like above
and was very puzzled what is the purpose of having two development
branches: 'master' and 'next'. Only later I realized that it is
necessary to give flexibility in making decisions of what should be
included in the next stable release and what may need more "cooking"
to prove their reliability and usefulness.
The first and second sentences are a bit disconnected here. I would
rather write the second one like this: "An example of such a change
can be a bug fix, which should be applied to all main branches."
Another thing is that I am not sure that the provided reason for doing
so ("Git is quite good at merges") is good enough. It can be said that
Git is quite good at cherry-picking too. Yet, we use merge, because it
allows to deal with large number of patches easier. Merge can be easily
visualized and understood as every merge point means that all changes
before it are included. However, to being able use merge, the developer
has to start from the oldest branch that will include this change. This
is a clear restriction over the anarchic nature of cherry-picking (where
you can introduce a change to an arbitrary branch and then cherry-pick
to others), but it pays off in the long run by better maintainability of
the project. Thus the recommended practice is a strong preference to use
merge over cherry-picking. It does not mean that cherry-picking should
be completely excluded. Occasionally, it may be useful.
IMHO, expressions such as "a quick moment of thought reveals..." is
more suitable for blogs than for serious documentation.
Perhaps, it is worth to note here that a non-trivial fixes can be
implemented as topic branches, which starts from the oldest branch
that needs them.
There is a far more important reason to use topic branches than ecstatic
pleasure from being able to see related changes grouped together in the
history. The main reason to use topic branches is to facilitate parallel
development. Though the idea that anyone commit to the main development
branch ('master') is very appealing due to its simplicity, it leads to
problems down the road. Namely, not all good sounding ideas turns out
good in reality.
In the workflow where everyone commits to 'master' there are only two
ways to deal with that. The first approach is not let developers to
commit their changes until they have completely finished their work
and passed all tests and code-review, and their work deemed important
enough to be included in the next feature release. The second approach
is to commit their work in progress in the hope that it will succeed,
and if not then to rollback changes.
Neither of these two approaches is satisfactory, especially for large
projects. The first approach means that developers are under a great
stress due to inability to save their work in progress, they accumulate
a huge patch, which is very difficult to review, often include some
other changes unrelated to the stated goal, and makes the history of
the project nearly useless for bisecting (linkgit:git-bisect[1]) when
it comes to finding a regression. The second approach means that the
project history gets contaminating with a great number of changes that
eventually didn't work out. Moreover, reverting changes that are belong
to some failed work may extremely difficult as other changes intervene
with them. So, this reverting is hardly ever done completely in practice
if it is done at all, which leads to a lot of garbage in the source
code. Obviously, the history of this project is completely useless for
bisecting as many commits do not really work if they are compiled at
all. Also, this approach leads to an extremely long stabilization period
as it is determined by the time when slowest going work will be in good
shape for release.
Using topic branches immune to that problem as feature are included
into 'master' when they are ready. Moreover, feature branches unless
they are "publish" can go through cycles of testing, review, and
interactive rebasing to edit and improve individual commits. Thus
the finally published history is clean and easy to bisect.
I think it would make sense to name them here.
Perhaps, it would be better to say:
"They are distinguished by the ability to propagate merges."
However, this is not the only distinguish between them. Besides, I am
not sure how this one is connected with the rest of the paragraph:
IMHO, it would be better:
s/the main history/the official history of the project/
Would that entrench the wrong idea that one needs to do 'pull'
habitually? And the habitual 'pull' results in habitual 'merge'.
This is nitpicking, but you cannot use 'pull' to send changes. However,
I suppose you meant to make your repository available for other people
to pull from it.
After reading "mixture of the two:" above, I expected these two being named,
but instead I can see three points. So, it is confusing.
Maybe, git pull --rebase is better advice here as it will also fetch
the latest changes from the upstream.
Could it not be any other reason besides renames? Maybe it is better to
drop "because of renames" here.
Because the word "index" is often used in Git in the different meaning
(a.k.a cache), I would re-write this sentence to avoid confusion as:
"`git am -3` will use information contained in index lines of patches"
If I did not know how git am -3 works, reading this would make me think
that git am somehow manage to figure out a common ancestor (commit),
while it uses index lines of the patch to learn the identity of the blob
that was used as the starting point to create the patch, and if this
blob is available locally, git am -3 performs 3-way merge.
So, "reconstruct a merge base" is hardly appropriate here.
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
