maintainers: require GitHub handle (documentation)#437469
maintainers: require GitHub handle (documentation)#437469wolfgangwalther merged 2 commits intoNixOS:masterfrom
Conversation
It's actually easier to request the user by ID and check the name matches, because the name is going to differ more significantly on a typo than the ID.
This documents the requirement for a github and githubId fields in maintainer handles. It does explicitly not enforce this requirement, yet, becaue there are still some maintainers without. Enforcment via tests/CI will happen in a separate step.
d7d58c1 to
c09d16c
Compare
|
@wolfgangwalther Thanks for all you work on maintainers! |
|
our entire CI hinges on wolfgang's maintenance |
|
Thanks for the feedback everyone. That's enough approvals for me to get this merged. |
MattSturgeon
left a comment
There was a problem hiding this comment.
I believe we also have some contributing docs that explicitly state a GitHub account is not required (to be a maintainer? contributor? I can't recall). If we're now enforcing that the attribute is set, we should rephrase that to avoid confusion.
-- #437085 (review)
The note I was thinking of was:
Lines 6 to 7 in 422a2b5
...From the start of CONTRIBUTING.md.
That's still correct, even after this and #437085; you don't need to be a maintainer to contribute. Although, I wonder if it should now have a caveat, for clarity?
Regardless, that's tangential. This PR is about the maintainer docs, not contributing in general.
I see what you mean. I think it would not actually help clarity if we mentioned the fact that you need a github account to be a maintainer. That's because at this stage, for the very first contribution, you don't even know what a maintainer is. If you're really in the situation that you don't want / have a github account, reading up on the maintainer thing might lead you to believe that you couldn't propose the addition of a new package. But that's not true, you could do that, somebody else would just have to commit to maintaining it (and opening the actual PR). Thus.. I'd say it's probably more confusing than helpful to mention it there, at least that's how I feel. |
I can get on board with this perspective. I see how a caveat could be more confusing than helpful, especially for someone still fuzzy on "maintainer" vs "contributor". A counter argument might be that I think you could refute my argument by saying that, regardless of how it is actually used in practice, it is written for new contributors. |
Currently, serious contribution is more or less impossible when being barred from maintainer rights. Without a corresponding clarification in the documentation, committers are likely to refuse PRs involving new packages just because there is no maintainer entry in One example would be the unrolling of git sources involving submodules in order to improve source fetching. Putting all the necessary logic into the package which includes such a git repository is inferior with respect to maintenance and code reuse, so a better solution is to put it into a separate package that can be used by other packages. In this case, @GaetanLepage split the contribution it out into a separate package and generously took the maintenance responsibility. However, this cannot possibly be expected for every contribution, and for the resulting Another example are the link paths in In both exemplary cases, limiting contribution to what is possible without adding new packages leads to obvious quality defects. Therefore, I would argue that contributing without maintaining is a rather artificial idea, not really congruent with real life needs. |
This documents the requirement for a github and githubId fields in maintainer handles. It does explicitly not enforce this requirement, yet, becaue there are still some maintainers without.
Enforcment via tests/CI will happen in a separate step, see #437085.
Please keep the discussion in this PR about the specific wording of the changes. Any discussion about "should we do this at all", is better done in #437085, where it was already started. We don't want to split this discussion among different PRs. Any such comment will be marked as "off-topic" here.
From the big numbers of approvals in #273220 and #437085, I conclude that we do want to do this. This PR follows up on what has essentially already been decided by the community, but has been blocked so far in implementation due to existing maintainer entries.
Things done
Add a 👍 reaction to pull requests you find important.