bpo-33671: Add support.MS_WINDOWS and support.MACOS by vstinner · Pull Request #7800 · python/cpython (original) (raw)

@vstinner It seems to me there are a number of issues here. Let me see if I can articulate them.

  1. Independent of what we do for master, I really don't think a change like this should be backported to maintenance branches (3.7, 3.6, much less 2.7 - although that's not my call). As the devguide puts it: "The only changes allowed to occur in a maintenance branch without debate are bug fixes." I would add "documentation fixes". "Also, a general rule for maintenance branches is that compatibility must not be broken at any point between sibling minor releases (3.5.1, 3.5.2, etc.). For both rules, only rare exceptions are accepted and must be discussed first." So we clearly make exceptions for security fixes and possibly platform changes (new OS releases, etc) usually with discussions. And we are perhaps a little bit looser with the tests part of the branches, so, for example, accepting without a lot of discussion small changes to regrtest that bring meaningful improvements to running tests with a very small risk of introducing a downstream user incompatibility and without requiring a change in developer workflow. But I don't think this proposed change fits into any of those categories. At best, it doesn't introduce any new errors. But ...
  2. it does force unnecessary changes on all maintainers and inspectors of the code base. Something as simple as grepping the code base for sections relevant to a particular platform now has to change and people now have to know to do that, for example, grepping for 'darwin', something I do a lot, isn't sufficient anymore. And they would need to know to use these new spellings when adding or changing tests, ...
  3. something they would hardly know about unless they stumble across it by accident (like I did) or examine the changed code base and start wondering what the difference is between the old spellings and the new. At the moment, as far as I know, there is no mention of this major change on the bug tracker at all so you can't search there for it; the only discussion is here on a merged PR that is attached to a totally unrelated 3.8-only feature. That doesn't seem right.
  4. So if the changes remain in master and are not backported, it seems like we have added an unnecessary maintenance burden by now preventing future automatic backports in dozens of files simply because of the changes in spellings that add no new functionality nor solve any existing bugs, simply adding another way to express something that might have been slightly more readable if we had used it from day one but in the end comes down to a matter of taste.
  5. (Although I would question whether:
 -        if sys.platform in ('linux', 'darwin'):
 +        if support.MACOS or sys.platform == 'linux':

a change being proposed in a follow-on PR is more readable, or even, were it available:

 +        if support.MACOS or support.LINUX:

)

So, I honestly don't know what the right thing to do for master is. It's not my call for 3.8 as I'm not wearing the release manager hat for it. But as a fellow core developer I think such a change deserves more exposure and possibly discussion before we follow it to its logical conclusion and people start proposing to make more wholesale changes throughout the code base (as they already are). And as a core developer, I would not be unhappy if it were reverted from master.

With my 3.7 and 3.6 release manager hat on, I'm a strong -1 on backporting unless someone can come up with a really compelling case to do so.