On Monday, 20 March 2017 at 12:25:22 UTC, deadalnix wrote:
Because a picture is clearer than a thousand words:
What this tells me is that the default way git-log presents
history is not very useful. Consider this presentation of the
same information:
08ae52d8 The Dlang Bot: Merge pull request #5231 from
RazvanN7/Update_generated
- c6480976 RazvanN7: Updated posix.mak makefile to use
../tools/checkwhitespace.d
1181fcf7 The Dlang Bot: Merge pull request #5239 from
sprinkle131313/ignore-vscode-lib
- f1b8d0d4 sprinkle131313: Add temp/tmp folder to gitignore.
- b67bf9d1 sprinkle131313: Add vscode folder and lib files to
gitignore.
0b41c996 The Dlang Bot: Merge pull request #5242 from
wilzbach/fix-lref-links
- 090d5164 Sebastian Wilzbach: Fix links from $(LREF $(D ...)) ->
$(LREF ...)
f2a019df The Dlang Bot: Merge pull request #5241 from
MartinNowak/merge_stable
- a6cb85b8 Sebastian Wilzbach: Add @safe to std.regex unittest
- ad70b082 Martin Nowak: Merge remote-tracking branch
'upstream/stable' into merge_st…
- 694dd174 Stefan Koch: Merge pull request #5167 from
DmitryOlshansky/fix-freeform-…
- 62cf615d Dmitry Olshansky: Fix issue 17212 std.regex
doesn't ignore whitespace …
5b07bd59 Sebastian Wilzbach: [BOOKTABLES]: Add BOOKTABLE to
stdx.checkedint (#5238)
75059373 Jack Stouffer: Merge pull request #5225 from
wilzbach/booktable-std-utf
In particular, the origin commit of a branch is often not
interesting; only the list of commits that are on one branch and
aren't on another are.
Anyway, personally I don't think there is a severe problem in
dire need of fixing with the git log excerpt you pasted. If
you're interested in looking at changes to the master branch,
look for asterisks in the first column.
In addition there are a bunch of practical issues with this way
of doing things.
There seem to be more practical issues with the opposite approach.
First there is no given that any intermediate state is sound,
or even builds at all. That makes it very hard to bissect
anything.
Bisecting D is not something that can be reasonably done by
looking at just one repository's history anyway; this is why we
have D-dot-git and Digger. Either way, for pull requests that
make non-trivial changes or additions, you will need to descend
into the pull request itself.
There are also a lot of errands and correction that are made
during review that are not that interesting to keep in the
project history. Knowing that someone did thing the A way and
then changed it the B way after review is more noise than
useful infos in the general case,
Agreed, this is one case where squashing is appropriate. However,
consider the worst-case scenarios where either merge strategy is
abused:
- If a pull request that should have been squashed has been
merged without squashing, the result is:
- Some clutter in the git history;
- Possible (but avoidable) complications when doing git-bisect
on a single repository, which you shouldn't be doing anyway.
- If a pull request that should not have been squashed has been
squashed while merging, the result is:
- Commit messages are lost and remain available only on GitHub.
- Any logical separation of changes that might have been
represented through separate commits is lost and remains
available only on GitHub.
- "git blame" becomes less useful because it can only lead to
the big blob of the squashed changes.
- "git blame" becomes less useful because in some situations it
loses its ability to track moved code, which should and often is
done in separate commits.
- Bisection becomes more difficult because it is no longer
easily possible to dive into a PR, as has been occasionally
necessary.
In general, I am not opposed to giving reviewers the option to
merge pull requests with squashing, assuming we can all agree to
not abuse it and only use it for PRs where there nothing useful
can be gained by preserving the multiple commits as they are;
however, their words and actions have shown that this doesn't
seem to be an attainable point of agreement.
