Introduce a structure for .github/CODEOWNERS #137498
Conversation
|
Ah, ranges with |
There was a problem hiding this comment.
ISTM that there are different possible approaches, each with pros and cons. Most of the entries seem to be about stdlib modules, built-in objects/functions1, and the interpreter core. The other sections (with the exception of Platform Support) have fewer entries, and could possibly be grouped together in a single "Others".
I've seen tools that can inspect CODEOWNERS files, but I'm not sure if there is something that can detect unintentional changes between the old and the new version (I think I spot at least one -- contextlib). If you can find something it would be good to check that the owners are still the same.
This still looks like an improvement compared to the previous version, even in its current form.
Footnotes
-
FWIW in the docs both the built-in objects/functions and stdlibg modules are grouped under the "library" section, whereas here some objects are under Interpreter Core and others under Standard Library. ↩
.github/CODEOWNERS
Outdated
| **/*pystate* @ericsnowcurrently @ZeroIntensity | ||
| **/*preconfig* @ericsnowcurrently | ||
| # Context variables & HAMT | ||
| **/contextvars* @1st1 |
There was a problem hiding this comment.
This used to be **/*context* (line 32 of the old file), which matched Lib/contextlib.py, but now contextlib doesn't seem to be matched by anything
There was a problem hiding this comment.
I forgot to put this in the PR description. I think this was a mistaken glob that was over-expansive -- the PR (#5276) seemed to only reference context variables, rather than context managers.
@1st1 -- Yury, do you want to be listed as a codeowner for contextlib/context managers, or only for context variables?
.github/CODEOWNERS
Outdated
| /Lib/test/test_html*.py @ezio-melotti | ||
| /Tools/build/parse_html5_entities.py @ezio-melotti | ||
| # Libssl / TLS | ||
| **/*ssl* @gpshead @picnixz |
There was a problem hiding this comment.
This, IDLE, and hashing could go into Standard Library as well.
There was a problem hiding this comment.
See the 'unresolved questions' bit of the PR description. Hashing & TLS/SSL seem broader than just the stdlib modules, and IDLE seems different than being 'just' a regular stdlib package.
Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: Ezio Melotti <[email protected]>
StanFromIreland
added
infra
|
I am currently on a very bad network for today so I can only quickly reply: SSL and hashing can go under stdlib because none of them are built-ins. However hashing for Python objects (not cryptographic hashes) should go under core. Everything related to cryptography can go under a subsection of library named cryptographic primitives and applications (SSL is too much for being a primitive imo and it allows us to put more stuff related to cryptography in the future). |
Co-authored-by: Hugo van Kemenade <[email protected]>
|
|
||
|
|
||
| # ---------------------------------------------------------------------------- | ||
| # Other / Misc (Tools, Programs, Integration, etc) |
There was a problem hiding this comment.
| # Other / Misc (Tools, Programs, Integration, etc) | |
| # Other / Misc (Tools, Programs, Integration, etc.) |
| # Exclusions from .github/CODEOWNERS should go at the very end | ||
| # because the final matching pattern will take precedence. |
There was a problem hiding this comment.
Should this be a top-level section, listed in the file structure at the top?
Co-authored-by: Ezio Melotti <[email protected]>
cc @ezio-melotti @hugovk
The CODEOWNERS file is getting a little disorganised. This PR is an attempt to add more of a structure & grouping, so future entries are less slapdash. For example, I was unsure the best place to add new lines in #137483.
I've organised the file by 'topic area': CI, build systems, core interpreter, docs, misc, platforms, and stdlib. Entries are kept alphabetically within each top-level section, and are grouped/subtitled with a comment.
I have tried to ensure that thre are no behavioural changes to
CODEOWNERSfrom this PR, but there is one exception -- I've removed the**/*cgi*entry ascgi.pyetc was removed in 3.13.Unresolved thoughts:
A