On 07Dec2018 1340, Terry Reedy wrote:
Simple bugfix example:
<commit 1> Add test to test_mod that fails with TwinkleError.
Posted to issue by Joe Blow.
<commit 2> Make new test pass using the 'underhand' strategy.
The split above is not really necessary, but PR 10245 squashed changes
to 52 files of 15 file types into one initial commit.
https://github.com/python/cpython/pull/10245/commits/17ba155a7b794926ce705ee0e2af787fbd2996e6
View Files displays them alphabetically. Jump to ... lists them in the
same order, but includes the functions changed, so it is hundreds of lines.
I think this PR would have benefited from having perhaps 10 or more
initial commits, each with a comment about the group of files included.
In icon commit could have said something about the source and purpose of
the added icon files. One commit could have included the venv and test
changes that you want to review. An added message, as long as needed,
could have explained these particular changes.
This is great in theory, but if you look back at the original PR it
would have been 100+ commits (I was occasionally squashing and force
pushing). What you're really proposing is:
* do all the work using the git workflow (stream-of-consiousness commits)
* squash all the commits at the end
* re-expand the single commit into logical groupings of files and
re-commit them
So it's easy to sit back and imagine that I had all the perfect changes
ready to go and deliberately chose to make it harder to review, but
that's not the case at all. Making it sound like this is how development
works is really disparaging to people who find themselves not producing
perfect commit histories.
And let's be honest, there's no good tooling for turning a series of
interdependent commits into a smaller set of sensible ones. Squashing at
least gets rid of the changes that were reverted as part of the entire
PR, and if you then just want to split by file you're really just asking
for extra work. If someone had bothered to say "I'll review the parts of
it that are relevant to me if you can split them out" then I would have,
but nobody (in public) showed any interest in reviewing the changes at
all - I had some private reviews done by colleagues at work, who weren't
so demanding about it.
I review as many PRs as I send these days (maybe more? I'm not
counting), and I always try to make an effort to have mercy towards
people to save them having to work extra hard just to make my life a
little bit easier. It grates to have had an incremental change visible
in public for over two months, have it be totally ignored the entire
time, and then have to defend something that I already did. Luckily I
get paid work time for doing this; really can't see why I'd want to
suffer through this for free...
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
