[Python-Dev] I reverted "Add Windows App Store package" change
Steve Dower
steve.dower at python.org
Fri Dec 7 18:06:25 EST 2018
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...
More information about the Python-Dev
mailing list