[Python-Dev] I reverted "Add Windows App Store package" change
Hi,
I had to revert a change since it broke buildbots. Sadly, I don't have
the bandwidth to investigate the failures and try to fix the change
:-(
A large change has been pushed into the 3.7 and master branches to
"Add Windows App Store package":
"Release Windows Store app containing Python"
https://bugs.python.org/issue34977
These changes broke all Windows buildbots:
"Almost all Windows buildbots are failing to compile"
https://bugs.python.org/issue35437
So I reverted these two changes.
It's a large change which mostly add new files, but also make changes
in different files:
---
commit 468a15aaf9206448a744fc5eab3fc21f51966aad
Author: Steve Dower
Date: Thu Dec 6 21:09:20 2018 -0800
bpo-34977: Add Windows App Store package (GH-10245)
.azure-pipelines/windows-appx-test.yml | 65 +++
.gitattributes | 1 +
Doc/make.bat | 2 +
Lib/test/test_pathlib.py | 2 +-
Lib/test/test_venv.py | 1 +
Lib/venv/__init__.py | 49 +-
.../2018-10-30-13-39-17.bpo-34977.0l7_QV.rst | 1 +
PC/classicAppCompat.can.xml| Bin 0 -> 3978 bytes
PC/classicAppCompat.cat| Bin 0 -> 10984 bytes
PC/classicAppCompat.sccd | Bin 0 -> 18503 bytes
PC/getpathp.c | 8 +-
PC/icons/pythonwx150.png | Bin 0 -> 8187 bytes
PC/icons/pythonwx44.png| Bin 0 -> 2232 bytes
PC/icons/pythonx150.png| Bin 0 -> 8271 bytes
PC/icons/pythonx44.png | Bin 0 -> 2178 bytes
PC/icons/pythonx50.png | Bin 0 -> 2190 bytes
PC/launcher.c | 220 +++-
PC/layout/__init__.py | 0
PC/layout/__main__.py | 14 +
PC/layout/main.py | 612 +
PC/layout/support/__init__.py | 0
PC/layout/support/appxmanifest.py | 487
PC/layout/support/catalog.py | 44 ++
PC/layout/support/constants.py | 28 +
.../support/distutils.command.bdist_wininst.py | 25 +
PC/layout/support/filesets.py | 100
PC/layout/support/logging.py | 93
PC/layout/support/options.py | 122
PC/layout/support/pip.py | 79 +++
PC/layout/support/props.py | 110
{Tools/nuget => PC/layout/support}/python.props| 0
PC/pylauncher.rc | 6 +
PC/python_uwp.cpp | 226
PC/store_info.txt | 146 +
PCbuild/_tkinter.vcxproj | 6 +
PCbuild/find_msbuild.bat | 10 +
PCbuild/pcbuild.proj | 3 +
PCbuild/pcbuild.sln| 72 +++
PCbuild/python.props | 3 +-
PCbuild/python_uwp.vcxproj | 86 +++
PCbuild/pythoncore.vcxproj | 15 +
PCbuild/pythonw_uwp.vcxproj| 86 +++
PCbuild/venvlauncher.vcxproj | 85 +++
PCbuild/venvwlauncher.vcxproj | 85 +++
Tools/msi/buildrelease.bat | 13 +-
Tools/msi/make_appx.ps1| 71 +++
Tools/msi/make_cat.ps1 | 34 ++
Tools/msi/make_zip.proj| 9 +-
Tools/msi/make_zip.py | 250 -
Tools/msi/sdktools.psm1| 43 ++
Tools/msi/sign_build.ps1 | 34 ++
Tools/nuget/make_pkg.proj | 38 +-
52 files changed, 3053 insertions(+), 331 deletions(-)
---
Note: the commit message is a single line.
Now I have questions :-)
It seems like the change is "experimental":
https://bugs.python.org/issue34977#msg331267
"""
Making the *release experimental* as part of the next 3.7 update was
approved by Ned (over email), so I merged the build. As soon as we
snap for the RC I'll kick off an update and make the store page
public, and hopefully can promote it enough to get eyes on it.
Unfortunately, I discovered as part of a test submission that the
minimum Windows version matters, and it's a version that hasn't been
rolled out fully yet *because of some bugs*, so there may not be that
many people who can use it this first time. But that will *improve
over time*, and I'm sure I can find enough people to flush out issues
before the next release (anyone on the Windows Insiders program should
be fine
Re: [Python-Dev] I reverted "Add Windows App Store package" change
Thanks for fixing up the buildbots, but please be a little more thorough before making publicly incorrect statements. The change is 99% adding new files that are not part of Python, but participate in the build for Windows (and are available and incredibly useful for everyone). These are essentially zero-impact. In the PR I pointed out exactly where to look for interesting changes and it didn't help get any traction :) The other changes are either in Windows-only files or tests. The one exception is venv, where they are in "if os.name=='nt'" blocks. I also pinged our venv expert a few times with no response. The PR was put up two *months* ago, not one week. In that time, it's been heavily tested by myself and a number of people who I *am* able to share the output of this with. I've also been chatting with the release manager for 3.7 about it and he's been on board (the words may have been "on your own head", but that's close enough to "on board") It didn't break all Windows buildbots. That said, it's totally my fault for merging and then not watching. Also for not submitting custom builds to all the buildbots (Can we still do that? I'm not seeing any UI right now... I did run a number of test release builds on the release machine, so I knew that was going to be okay.) Now, to answer your questions: releasing in a new package with slightly different semantics *has* to be somewhat experimental, because nobody can test it until it's been released. This isn't about iterating on this change in master, it's about getting broad public feedback on a release mechanism that currently does not exist. Historically, when changes to the part you point out have been extracted out from their context, they've been rejected on the basis that they aren't necessary. But now the original PR is broken (because the tests don't pass), and so there will never be a need. This time I decided to specifically point out numerous times where the interesting changes were and was not able to get reviews from bpo/github (I did get many reviews and some testing from others). As it happens, I split out many changes to do with pathconfig that came from this. You rejected them because they weren't necessary :) Most of the code is a Python script to do "make install" on Windows. A very common request is "how do I copy built files into the right place", and the answer has always been "it's complicated". Now we can measure how complicated it is in terms of lines of Python code, but at least the answer is "run this script". Yes, it could go into its own PR, but that runs into the context problem again. If I'm going to be forced to bypass review on a dependency just to make it possible to merge a real change, then they may as well go together. (The new script is Black formatted, so probably I could get Lukasz to approve it on its own? :) ) I hope that explains a bit better. People wait two months and more for simpler reviews, but part of me being a core developer is to accelerate that for Windows-targeted work. That's all I did here, and I'm happy with my reasoning. I've reposted the PRs at https://github.com/python/cpython/pull/11027 and https://github.com/python/cpython/pull/11028 with fixes for the one issue that Victor couldn't investigate. If someone can get a Windows buildbot to run against them that would be great (not you Zach - your buildbots were fine :) ). Cheers, Steve On 07Dec2018 0435, Victor Stinner wrote: > Hi, > > I had to revert a change since it broke buildbots. Sadly, I don't have > the bandwidth to investigate the failures and try to fix the change > :-( > > > A large change has been pushed into the 3.7 and master branches to > "Add Windows App Store package": > > "Release Windows Store app containing Python" > https://bugs.python.org/issue34977 > > These changes broke all Windows buildbots: > > "Almost all Windows buildbots are failing to compile" > https://bugs.python.org/issue35437 > > So I reverted these two changes. > > It's a large change which mostly add new files, but also make changes > in different files: > --- > commit 468a15aaf9206448a744fc5eab3fc21f51966aad > Author: Steve Dower > Date: Thu Dec 6 21:09:20 2018 -0800 > > bpo-34977: Add Windows App Store package (GH-10245) > > .azure-pipelines/windows-appx-test.yml | 65 +++ > .gitattributes | 1 + > Doc/make.bat | 2 + > Lib/test/test_pathlib.py | 2 +- > Lib/test/test_venv.py | 1 + > Lib/venv/__init__.py | 49 +- > .../2018-10-30-13-39-17.bpo-34977.0l7_QV.rst | 1 + > PC/classicAppCompat.can.xml| Bin 0 -> 3978 bytes > PC/classicAppCompat.cat| Bin 0 -> 10984 bytes > PC/classicAppCompat.sccd | Bin 0 -> 18503 bytes > PC/getpathp.c |
Re: [Python-Dev] I reverted "Add Windows App Store package" change
As a slight aside, 8 out of 8 buildbot messages on the PR look like false positives, and none of the true positives sent a message. What happened there? On 07Dec2018 0716, Steve Dower wrote: > Thanks for fixing up the buildbots, but please be a little more thorough > before making publicly incorrect statements. > > The change is 99% adding new files that are not part of Python, but > participate in the build for Windows (and are available and incredibly > useful for everyone). These are essentially zero-impact. In the PR I > pointed out exactly where to look for interesting changes and it didn't > help get any traction :) > > The other changes are either in Windows-only files or tests. The one > exception is venv, where they are in "if os.name=='nt'" blocks. I also > pinged our venv expert a few times with no response. > > The PR was put up two *months* ago, not one week. In that time, it's > been heavily tested by myself and a number of people who I *am* able to > share the output of this with. I've also been chatting with the release > manager for 3.7 about it and he's been on board (the words may have been > "on your own head", but that's close enough to "on board") > > It didn't break all Windows buildbots. > > That said, it's totally my fault for merging and then not watching. Also > for not submitting custom builds to all the buildbots (Can we still do > that? I'm not seeing any UI right now... I did run a number of test > release builds on the release machine, so I knew that was going to be okay.) > > Now, to answer your questions: releasing in a new package with slightly > different semantics *has* to be somewhat experimental, because nobody > can test it until it's been released. This isn't about iterating on this > change in master, it's about getting broad public feedback on a release > mechanism that currently does not exist. > > Historically, when changes to the part you point out have been extracted > out from their context, they've been rejected on the basis that they > aren't necessary. But now the original PR is broken (because the tests > don't pass), and so there will never be a need. This time I decided to > specifically point out numerous times where the interesting changes were > and was not able to get reviews from bpo/github (I did get many reviews > and some testing from others). > > As it happens, I split out many changes to do with pathconfig that came > from this. You rejected them because they weren't necessary :) > > Most of the code is a Python script to do "make install" on Windows. A > very common request is "how do I copy built files into the right place", > and the answer has always been "it's complicated". Now we can measure > how complicated it is in terms of lines of Python code, but at least the > answer is "run this script". Yes, it could go into its own PR, but that > runs into the context problem again. If I'm going to be forced to bypass > review on a dependency just to make it possible to merge a real change, > then they may as well go together. > > (The new script is Black formatted, so probably I could get Lukasz to > approve it on its own? :) ) > > I hope that explains a bit better. People wait two months and more for > simpler reviews, but part of me being a core developer is to accelerate > that for Windows-targeted work. That's all I did here, and I'm happy > with my reasoning. > > I've reposted the PRs at https://github.com/python/cpython/pull/11027 > and https://github.com/python/cpython/pull/11028 with fixes for the one > issue that Victor couldn't investigate. If someone can get a Windows > buildbot to run against them that would be great (not you Zach - your > buildbots were fine :) ). > > Cheers, > Steve > > On 07Dec2018 0435, Victor Stinner wrote: >> Hi, >> >> I had to revert a change since it broke buildbots. Sadly, I don't have >> the bandwidth to investigate the failures and try to fix the change >> :-( >> >> >> A large change has been pushed into the 3.7 and master branches to >> "Add Windows App Store package": >> >> "Release Windows Store app containing Python" >> https://bugs.python.org/issue34977 >> >> These changes broke all Windows buildbots: >> >> "Almost all Windows buildbots are failing to compile" >> https://bugs.python.org/issue35437 >> >> So I reverted these two changes. >> >> It's a large change which mostly add new files, but also make changes >> in different files: >> --- >> commit 468a15aaf9206448a744fc5eab3fc21f51966aad >> Author: Steve Dower >> Date: Thu Dec 6 21:09:20 2018 -0800 >> >> bpo-34977: Add Windows App Store package (GH-10245) >> >> .azure-pipelines/windows-appx-test.yml | 65 +++ >> .gitattributes | 1 + >> Doc/make.bat | 2 + >> Lib/test/test_pathlib.py | 2 +- >> Lib/test/test_venv.py | 1 + >> Lib/venv/__init__.py |
Re: [Python-Dev] I reverted "Add Windows App Store package" change
> On 07Dec2018 0716, Steve Dower wrote: > > It didn't break all Windows buildbots. > > > > That said, it's totally my fault for merging and then not watching. Also > > for not submitting custom builds to all the buildbots (Can we still do > > that? I'm not seeing any UI right now... I did run a number of test > > release builds on the release machine, so I knew that was going to be okay.) As to the breakage on my buildbot (https://buildbot.python.org/all/#/workers/12), it seems to be caused by not having VS2017 (my guess due to missing C++ headers). Unless something has changed recently, the guides still state the building with VS2015 is OK and I would like to keep my buildbot testing at the *minimum* required components for building (hence the VS9.0 builder) to keep us honest with any changes. I have no problem adding VS2017 for newer Python versions, but I think is build scripts always chose the latest version of the build tools thus making testing with older toolsets impossible. -- Jeremy Kloth ___ Python-Dev mailing list [email protected] https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] I reverted "Add Windows App Store package" change
On Fri, Dec 7, 2018 at 9:17 AM Steve Dower wrote: > Also for not submitting custom builds to all the buildbots (Can we still do > that? I'm not seeing any UI right now... I did run a number of test > release builds on the release machine, so I knew that was going to be okay.) The UX is not great, but you can trigger a custom builder build by pushing to the `buildbot-custom` branch of python/cpython. Eventually, time will materialize to make it possible for core devs to trigger builds from branches on their personal fork since we do have proper auth now. -- Zach ___ Python-Dev mailing list [email protected] https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] I reverted "Add Windows App Store package" change
Le ven. 7 déc. 2018 à 16:42, Steve Dower a écrit :
> As a slight aside, 8 out of 8 buildbot messages on the PR look like
> false positives, and none of the true positives sent a message. What
> happened there?
I don't know why the PR didn't get notifications about the regression.
I got something more than 20 emails in my buildbot mail box :-)
Let me have a look:
https://github.com/python/cpython/pull/10245#issuecomment-445184041
There are multiple messages related to the 'custom' build: Pablo's
created a custom build ("buildbot-custom" Git branch) which reverted
your PR in master. It seems like Pablo's buildbot which sends
notifications to PRs decided that his commit is attached to your PR.
You can ignore all notifications from the "custom" build. The GitHub
notifications are still experimental. If someone is interested, see
how you can enhance these notifications ;-)
s390x Debian 3.7 failed on "git clone". Well, that's a random network
issue. We are working on trying to make this specific issue quiet:
https://github.com/python/buildmaster-config/pull/69
Anyone is welcome to help to enhance the Python CIs, there are plenty
of things to enhance ;-) Buildbots are complex beast, but the
situation should be better nowadays than 2 years ago. The number of
random failure is quite low.
Maybe random failures are more visible today because notifications are
sent on state change (basically when the color changes). Two years
ago, all buildbots were always red (fail), so we never get any
notification :-)
I'm still fixing random failures every week: test_eintr, test_socket, etc.
Victor
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] I reverted "Add Windows App Store package" change
Le ven. 7 déc. 2018 à 16:16, Steve Dower a écrit : > The other changes are either in Windows-only files or tests. The one > exception is venv, where they are in "if os.name=='nt'" blocks. I also > pinged our venv expert a few times with no response. Yeah, the lack of review is an issue in Python, I'm well aware of that... But I guess that it would be easier to get a review on a change modifying a single file (venv) than one PR modifying 52 files? Usually, I'm scared by giant PRs and I just skip them :-) > The PR was put up two *months* ago, not one week. Oh, wait... I read Oct 30 and I counted 7 days... sorry :-) November becomes December, not October. Sorry about that. > That said, it's totally my fault for merging and then not watching. That's fine. A revert doesn't mean that your change is wrong. The only intent is to repair the CI as soon as possible to spot other regressions, and give us more time to design the proper fix: https://pythondev.readthedocs.io/ci.html#revert-on-fail > (Can we still do that? I'm not seeing any UI right now... I did run a number > of test > release builds on the release machine, so I knew that was going to be okay.) The current process is described at: https://devguide.python.org/buildbots/#custom-builders > Historically, when changes to the part you point out have been extracted > out from their context, they've been rejected on the basis that they > aren't necessary. I don't think that there is a general rule. It's usually decided on a case by case basis, and I know that each core dev has a different opinion on this question :-) I do prefer multiple small commits. I would be simpler if it would be possible to have a "patch serie": list of pull requests, or a single PR with multiple commits but don't squash them. > As it happens, I split out many changes to do with pathconfig that came > from this. You rejected them because they weren't necessary :) I'm sorry, I don't recall, which changes? > I hope that explains a bit better. It does, thanks :-) Victor ___ Python-Dev mailing list [email protected] https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
[Python-Dev] Summary of Python tracker Issues
ACTIVITY SUMMARY (2018-11-30 - 2018-12-07) Python tracker at https://bugs.python.org/ To view or respond to any of the issues listed below, click on the issue. Do NOT respond to this message. Issues counts and deltas: open6904 (+24) closed 40284 (+50) total 47188 (+74) Open issues with patches: 2743 Issues opened (49) == #35364: Datetime âfromtimestamp()â ignores inheritance if timezone https://bugs.python.org/issue35364 opened by Delgan #35366: Monkey Patching class derived from ctypes.Union doesn't work https://bugs.python.org/issue35366 opened by rvijayc #35368: [2.7] Make PyMem_Malloc() thread-safe in debug mode https://bugs.python.org/issue35368 opened by vstinner #35370: Provide API to set the tracing function to be used for running https://bugs.python.org/issue35370 opened by fabioz #35374: Windows doc build does not find autodetected hhc.exe https://bugs.python.org/issue35374 opened by chrullrich #35376: modulefinder skips nested modules with same name as top-level https://bugs.python.org/issue35376 opened by rdb #35377: urlparse doesn't validate the scheme https://bugs.python.org/issue35377 opened by devkral #35378: multiprocessing.Pool.imaps iterators do not maintain alive the https://bugs.python.org/issue35378 opened by pablogsal #35379: IDLE's close fails when io.filename set to None https://bugs.python.org/issue35379 opened by rhettinger #35381: Heap-allocated posixmodule types https://bugs.python.org/issue35381 opened by eelizondo #35383: lib2to3 raises ParseError on argument called "print" https://bugs.python.org/issue35383 opened by n_rosenstein #35385: time module: why not using tzname from the glibc? https://bugs.python.org/issue35385 opened by vstinner #35387: Dialogs on IDLE are accompanied by a small black window https://bugs.python.org/issue35387 opened by wordtech #35388: _PyRuntime_Initialize() called after Py_Finalize() does nothin https://bugs.python.org/issue35388 opened by vstinner #35391: threading.RLock exception handling while waiting https://bugs.python.org/issue35391 opened by Omer Bartal #35392: Create asyncio/sockutils.py https://bugs.python.org/issue35392 opened by asvetlov #35393: Typo in documentation https://bugs.python.org/issue35393 opened by autom #35394: Add __slots__ = () to asyncio protocols https://bugs.python.org/issue35394 opened by asvetlov #35397: Undeprecate and document urllib.parse.unwrap https://bugs.python.org/issue35397 opened by steven.daprano #35398: SQLite incorrect row count for UPDATE https://bugs.python.org/issue35398 opened by Montana Low #35399: Sysconfig bug https://bugs.python.org/issue35399 opened by neyuru #35400: PGOMGR : warning PG0188: https://bugs.python.org/issue35400 opened by neyuru #35401: Upgrade Windows and macOS installers to use OpenSSL 1.1.0j / 1 https://bugs.python.org/issue35401 opened by ned.deily #35402: Upgrade macOS (and Windows?) installer to Tcl/Tk 8.6.9.1 https://bugs.python.org/issue35402 opened by ned.deily #35403: support application/wasm in mimetypes and http.server https://bugs.python.org/issue35403 opened by pmpp #35404: Document how to import _structure in email.message https://bugs.python.org/issue35404 opened by charlax #35409: Async generator might re-throw GeneratorExit on aclose() https://bugs.python.org/issue35409 opened by vxgmichel #35410: copy.deepcopy does not respect metaclasses with __deepcopy__ i https://bugs.python.org/issue35410 opened by elibixby #35412: test_future4 ran no test https://bugs.python.org/issue35412 opened by vstinner #35413: test_multiprocessing_fork: test_del_pool() leaks dangling thre https://bugs.python.org/issue35413 opened by vstinner #35414: A reference counting bug in PyState_RemoveModule() https://bugs.python.org/issue35414 opened by ZackerySpytz #35415: fileno argument to socket.socket is not validated https://bugs.python.org/issue35415 opened by Dima.Tisnek #35416: Fix potential resource warnings in distutils https://bugs.python.org/issue35416 opened by Tiger-222 #35417: Double parenthesis in print function running 2to3 in already c https://bugs.python.org/issue35417 opened by jondaa #35419: Thread.is_alive while running Process.is_alive causes either p https://bugs.python.org/issue35419 opened by Hexorg #35420: how to migrate a c-extension module to one that supports subin https://bugs.python.org/issue35420 opened by mattip #35422: misleading error message from ssl.get_server_certificate() whe https://bugs.python.org/issue35422 opened by cedricvanrompay #35423: Signal handling machinery still relies on "pending calls". https://bugs.python.org/issue35423 opened by eric.snow #35424: multiprocessing.Pool: emit ResourceWarning https://bugs.python.org/issue35424 opened by vstinner #35425: test_eintr fails randomly on AMD64 FreeBSD 10-STABLE Non-Debug https://bugs.python.org/issue35425 opened by vstinner #35426: test_signal.test_interprocess_signal() race co
Re: [Python-Dev] I reverted "Add Windows App Store package" change
On 12/7/2018 11:31 AM, Victor Stinner wrote: I would be simpler if it would be possible to have a "patch serie": list of pull requests, One can make an 'index issue' with multiple dependencies, each with a PR. I do this for multiple independent changes to a module or related modules. or a single PR with multiple commits but don't squash them. Already possible on Github. If one clicks [View Changes], multiple commits in a PR are combined for display purposes into a net PR diff. But the individual commits can be examined and reviewed individually and are not squashed into one by github (and should not be by authors) until the merge into cpython. Typically, authors effectively 'squash' all initial changes into one commit, make a PR, and only add additional commits in review. But authors *can* split the initial editing into multiple commits with an eye to easing review -- and record information for future maintainers. Simple bugfix example: Add test to test_mod that fails with TwinkleError. Posted to issue by Joe Blow. 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. -- Terry Jan Reedy ___ Python-Dev mailing list [email protected] https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] I reverted "Add Windows App Store package" change
On 07Dec2018 1340, Terry Reedy wrote: Simple bugfix example: Add test to test_mod that fails with TwinkleError. Posted to issue by Joe Blow. 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 [email protected] https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
