Am 12.07.2015 um 00:17 schrieb Jason Moore:
FYI: We had a discussion at SymPy on how to synchronize with master and
decided that we no longer want to allow or encourage rebasing after a pull
request is made to the main SymPy repo.
I think that's overstated.
I see some reasons against merging:
A) A history with merges is harder to read. You need to track multiple
parallel developments, and that's particularly hard if merge conflicts
were resolved (because some of the code you see didn't get into master,
at least not in the form it's committed initially).
B) The results from `git bisect` become harder to read if a bug stems
from the combination of two changes that were made in parallel branches:
the bisect will point at the merge commit, but the actual problem
happened in some earlier commit.
The more parallel branches in existence, the more prominent this will
become.
The main reason is that we stall too many new contributors by forcing them
to do too much git kung fu.
This problem can be mitigated via interactive rebasing, but I agree it's
an extra hoop to jump through and cannot be asked of everyone.
> But there are additional important reasons too:
- Once a pull request is "public" the history should not change because
other people may have pull it.
Does anybody ever pull from a pull request?
I can see that happening when collaborating on a fix, but it's not
something affecting newbies.
- Rebasing with merge conflicts is much more difficult
Only if you try to rebase everything in a single go. You may end up
resolving the same conflict over and over for each commit, and that can
get confusing very quickly, and it's also annoying and a lot of needless
work.
However, there is a process that's actually quite easy:
1) `git rebase --interactive` first; this is a rebase *within your
working branch*.
I also squash commits that were a continuation of previous work. I.e. if
I have a sequence of 1.code - 2.reformat - 3.code - 4.bugfix -
5.reformat - 6.code - 7.bugfix - 8.reformat, I like to rearrange this to
1.code - 3.code - 4. bugfix - 6.code - 7.bugfix - 5.reformat -
7.reformat, and then I squash this into two commits, one with 13467 for
the coding and 57 for reformatting.
If this still is confusing, this can be broken down into a series of
primitive steps: (a) swap two commits, leaving all others alone, (b)
merge two adjacent commits, leaving all others alone; repeat until the
history is rewritten in a way that's easy to read and review.
2) `git fetch master && git rebase master`. This will expose the merge
conflicts, but now it's far less commits, and it's also giving you much
less pain because most conflicts will apply just in one commit.
- Reviewers are no longer able to see commit by commit changes in a review
process.
Only if you squash everything into a single commit.
Reordering and squashing commits makes sense if some work jumped between
multiple things, such as reformatting, fixing docstrings, and actual
code changes (and if the code changes touched multiple loosely-related
points, it may make sense to keep these in separate commits anyway).
- Github does not manage the diffs in a PR review if a rebase happens.
You mean the "click here to see outdated diffs" thing?
Sure, part of the discussion are removed one more click, which means
that people won't know to look there and usually won't find them.
However, if a discussion is relevant for posteriority, it should have
been merged into the commit in some way, either into the commit comment
or in a code comment or docstring. After all, people who're looking at
`git annotate` or `git log` aren't going to see these discussions either.
If you want to rebase then you should do it before you make a pull request,
either locally or on your fork's branch.
Oh, now I see what's the concern - indeed, rebasing on the main repo is
bad, very bad, and a real no-go. But that's a non-issue!
For pull requests, they're on separate branches, and people don't work
on the PR branch of anybody else (unless they explicitly have agreed to
cooperate in that way, in which case the work branch should indeed not
be rebased anymore unless all downstream collaborators agree).
Newbies don't even have the permissions to modify the main repo.
The one point where I see strong language against rebasing justified is
when merging into master on the main repo. This is merge, and merge
only, and should be written in bold letters into the project maintenance
guidelines.
For people who contribute pull requests, I do not think that applies.
YMMV, but I'd be interested to hear why.
Regards,
Jo
--
You received this message because you are subscribed to the Google Groups
"sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sympy+unsubscr...@googlegroups.com.
To post to this group, send email to sympy@googlegroups.com.
Visit this group at http://groups.google.com/group/sympy.
To view this discussion on the web visit
https://groups.google.com/d/msgid/sympy/55A22B0B.20405%40durchholz.org.
For more options, visit https://groups.google.com/d/optout.
