[IPython-dev] Pull request workflow...

[Ariel Rokem]

Hi Fernando and everyone,

Since we talked about this just yesterday in the context of nitime, I
thought I would pitch in with one more thought:

Do we think balance is the following?
>
> - From the proposer's side, rebase *before* you make your first pull
> request.  After that, don't worry too much about any more rebasings.
> That will give a reasonably clean starting point for the review, as
> well as ensuring that the branch applies cleanly.  For developers
> (like Min points out with his example) who are very comfortable with
> rebasing in-review they can do so (like he did), but we shouldn't
> *ask* that they do.
>
> - And no rebasing from the committer's side like I originally
> proposed, except in cases where significant criss-cross merges need to
> be cleaned up.
>
> And we could make the idea of an initial rebase 100% optional, only to
> be done by those who feel comfortable with git.  I know the word
> 'rebase' scares many new to git, and we don't want to put artificial
> barriers to contribution.  So it may be best to say that these are our
> suggestions for more advanced developers proposing complex merges, but
> that we will still accept any non-rebased pull request, as long as it
> doesn't merge *from trunk*.
>
>
In principle, this sounds balanced and good to me. However, after reading
the first email in this thread, I realized that what I hadn't considered
when we talked about this yesterday, is the difficulty posed to code
reviewers by the messed-up history caused by merging into a branch, rather
than rebasing.

What I am currently thinking is that the reviewer can decide whether the
merge causes the history to be so messy such that they cannot understand and
review the commits in the pull request. I think that this makes sense,
because if the history is messed up so badly that it can't be easily
reviewed now, it will only be more difficult to understand in a couple of
months, or in a year. This would be equivalent to some of the decisions that
reviewers make about code style and clarity. You might decide that the code
proposed is just too riddled with stylistic eye-sores to be merged (even if
it does what it is supposed to do) and ask a contributor to clean up the
code before pulling. On the other hand, you might decide to let a couple of
eye-sores slip by, in order to make a contributor's life a bit more easy. I
think that the same should apply to the git history that would result from
the pull. If it causes a slight criss-crossing in the history, that is easy
enough to figure out, let it by. If it actually makes review of the code
difficult, send a message to the contributor, preferably with some direction
on how to fix it and how not to do it again, much the same as you would for
a contribution that contains stylistic errors. Just a thought.

Cheers,

Ariel



How does this sound for our final take?
>
> Cheers,
>
> f
> _______________________________________________
> IPython-dev mailing list
> IPython-dev at scipy.org
> http://mail.scipy.org/mailman/listinfo/ipython-dev
>



-- 
Ariel Rokem
Helen Wills Neuroscience Institute
University of California, Berkeley
http://argentum.ucbso.berkeley.edu/ariel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/ipython-dev/attachments/20101012/d75044fb/attachment.html>