Thanks Brian for your nice summary! Let me complete it. Warning: I didn't look at https://github.com/python/cpython/pull/4819 at all. My email is a really general remark on the Python workflow. You in this email is not the OP, but "any contributor" ;-)
What contributors don't see is that regressions are very common in Python: bugs introduced by recent changes (feature or bugfix). Usually, the author of the change is gone when the regression pops up, and the core developers who merged the PR "must" handle the regression. And it's not about a single regression and you're done ever, you can get more. In the worst case, the fix for a regression introduced a second regression. And then you have to handle new backward compatibility issue when fixing a stable Python version :-( What happens usually is that some modules have no maintainer. Once you merge a single change in a module, boom! You are now the new maintainer for your entire life time! More and more people will ask you to look at their "very important" bug blocking their production and their very specific use case. You will get more and more reviews request: "since you merged a single change 12 months ago, I'm sure that you will love to review my precioooous bugfix! Come on, it's trivial!". Now you realize that you don't know the design of the module. You don't know its history. You know nothing, and the entire world now have very high expectation, since you merged an obvious and trivial change. It happens to me all the time. Most stdlib modules have no maintainer and past maintainers are gone for a long time. Merging a PR is very "expensive" for a core developer. There are very good reasons to not merge a PR. The status quo is much more comfortable for the core dev who decided to *not* be the one who merged your PR. I prefer to pretend that I know nothing about Python and only focus on the code area that I know the best, to reduce the risk of regression when merging a change. If you don't know well a module, the risk of missing a bug (breaking a specific code path) is way higher. The other problem is that it's very rare that a PR is reviewed by more than a single core developer. The core developer will be the only responsible person to handle the future incoming mess introduced by the "obvious and short bugfix". I exaggerate the severity of regressions to explain my point, but trust me, they are more likely than what you are thinking. Also, more contributors are around for 1 to 6 months, whereas core developers are usually around for 1 to 5 years. It's a different time scale in terms of responsibilities. Victor On Tue, Jun 29, 2021 at 7:21 PM Brian Curtin <br...@python.org> wrote: > > On Tue, Jun 29, 2021 at 10:38 AM <esmeraldagar...@byom.de> wrote: >> >> I just stumbled upon the following issue and subsequent pull request. It is >> a very small bugfix. There is currently a bug in Python and this pull >> request fixes it. It's not a new feature or an enhancement, it is a bugfix! >> Yet, it doesn't get reviewed, nor merged. And this has been going on since >> March 2017. Why not just merge this? It's not like it's huge or obstructing >> or obfuscating the current code base? There's always time to write an >> improvement or a complete rewrite of this module feature later for an >> upcoming minor release. But if there is currently a bug in Python and the >> bugfix is available - why doesn't it get merged? >> >> https://github.com/python/cpython/pull/4819 > > > For starters, the PR is closed in favor of another issue that has reviews and > a discussion, but even the smallest change like that requires a lot out of a > reviewer. Looking at that change, I don't personally know that it's correct, > so I'd have to take the time to figure out that it's correct. It includes no > tests, so I certainly don't trust that it's correct, so it looks incomplete > to me. > > Time is irrelevant here—there's no need to rush things because a change > appears small. What if that one line change is even more wrong than before? I > have merge access and if I just said "ah it's a small PR and it's been open > for a while, I'll just merge it for them," any change to Python has the > possibility to affect a huge amount of people. > > When I got the shutil.which feature merged, the PR had been open for I > believe 11 years and it was mostly complete in the initial patch outside of a > few small issues, and the change itself wasn't a lot of code. To just have > merged it because it was open for 11 years would have been the wrong thing to > do. It needed to cover some things it didn't initially cover, it needed tests > and documentation, and it wasn't merged until it was completed and properly > reviewed. > >> If this doesn't get fixed, doesn't that mean the Python review process is >> flawed? Sure, Python is an open source project and many people just work in >> their free time. But this doesn't really apply here, does it? The bugfix is >> available. Only a review is required. All this is happening while new >> features get added to Python with every new minor version. While the bug is >> allowed to live there. Please help me understand how this can happen. > > > "Only a review is required" is vastly understating the value of code reviews. > Almost anybody can write a one line fix, but is it the right fix? Does it > cover all of the cases it needs to? Is adding "manager_owned=False" correct > or should something else actually be done? Who knows and is available to > understand the impacts of this change? > > So does this mean the review process is flawed? I would say no, the _review > process_ is maybe working as expected—the linked PR was incomplete and wasn't > merged, another PR has come up, and there's discussion on it including a > comment about tests and one about familiarizing with the code. The process of > finding humans who are willing and able to do this work—currently for free—is > possibly broken, possibly working as expected, and overall is a significantly > harder problem to fix than most anything involved with open source software. > >> I love Python. No hard feelings. But this is really bugging me and I can't >> help but feel disappointed. > > > The good thing is that you paid nothing for this, so being disappointed is > something you can fix. If you would like more value out of it or to speed up > the process, you can provide your own reviews. Reviewing code is immensely > valuable and helps so many people—the core developers, the users, and > yourself. Alternatively, paying people to do the reviews is also possible. > >> _______________________________________________ >> Python-Dev mailing list -- python-dev@python.org >> To unsubscribe send an email to python-dev-le...@python.org >> https://mail.python.org/mailman3/lists/python-dev.python.org/ >> Message archived at >> https://mail.python.org/archives/list/python-dev@python.org/message/TTJAVTPF6RYO63GTBSTXFJG3IVCYPHXT/ >> Code of Conduct: http://python.org/psf/codeofconduct/ > > _______________________________________________ > Python-Dev mailing list -- python-dev@python.org > To unsubscribe send an email to python-dev-le...@python.org > https://mail.python.org/mailman3/lists/python-dev.python.org/ > Message archived at > https://mail.python.org/archives/list/python-dev@python.org/message/5PYTF4BEKS3H2AZX6ZQXKIQM5SYTOH7H/ > Code of Conduct: http://python.org/psf/codeofconduct/ -- Night gathers, and now my watch begins. It shall not end until my death. _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/LTTS76JH7SQR6STDVZXNRWDH3FPPRJXP/ Code of Conduct: http://python.org/psf/codeofconduct/
