Home Original page

[python-committers] [Core-mentorship] Regarding reviewing test cases written for tabnanny module

Guido van Rossum guido at python.org
Tue Apr 11 00:53:44 EDT 2017 
On Mon, Apr 10, 2017 at 8:55 PM, Martin Panter <vadmium+py at gmail.com> wrote:

> In this particular pull request, I think the submitter has rebased
> their commit, and force-pushed it. These days, I notice Git Hub seems
> to forget old commits pretty soon after you force-push the branch they
> are on. I don't think you can "unsquash" them retrospectively; you
> would need a copy of the old commits saved somewhere.
>
> Other times people add revised commits on top of their old commits,
> which would have been easier for me in this situation, but I suspect
> that makes it harder for the person pushing the final change if they
> have to squash it into a single commit. (I noticed the eventual commit
> message is often messy, redundant, automatically generated, etc.)
>

We have some of this in the mypy project (though it's a much smaller
project). The workflow that I like best (not all contributors use it) is

- When the contributor makes multiple local commits without pushing to the
PR, I recommend using --amend unless they have several commits that
actually are logically distinct and relevant to the reviewer. (--amend is
especially important when fixing lint bugs or typos).

- Please don't use --amend across pushes to the PR

- It's OK to just pull+rebase and then use git push -f, but honestly
pull+merge is fine too

- When responding to a review please NEVER use commit --amend, that's where
reviewing becomes painful

- It's up to the reviewer who merges to rewrite the commit message: the
reviewer usually has a much better sense for what info in the commit
message will still be interesting a month or a year from now than the
contributor. (Often just copying the original comment from the top of the
PR is adequate.)

In mypy I recently disabled merge commits in mypy because some other
committers sometimes accidentally used it, and the history becomes really
ugly. (Maybe Brett already did this for cpython.)

I do agree that Rietveld's workflow is usually much more convenient, with
the exception of what it does around merging or rebasing other diffs.

-- 
--Guido van Rossum (python.org/~guido)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-committers/attachments/20170410/7dd0f70d/attachment-0001.html>
More information about the python-committers mailing list