More flexible language for git vetted status for co-maintainers of existing projects

Created on 9 February 2023, almost 2 years ago

Problem/Motivation

Posting this after a discussion in slack with @greggles.

https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

This page has some language about applying for the git vetted role as a maintainer of an existing project, but from my reading of it, almost no-one would ever be eligible:

Can I apply using a project for which I am offering to become maintainer?
No, you cannot, except in the case you committed most of the code in a branch for that project.
This means that, for example, if you are co-maintainer of that project and you want to become maintainer (user with all the permissions on that project), or the new project owner, then you can use that project for the application, as long as there is a branch in that project with code committed mostly by you.

Can I apply using an issue branch I created for a project?
No, you cannot.
The reason is simply because these applications can require changes that aren't allowed in an issue branch, which is instead used to fix a particular issue in a project. For example, an issue created to change code that uses a deprecated function or method cannot be used to fix the content of the README.md file, while a review done for these applications could require you to change the content of that file.

"as long as there is a branch in that project with code committed mostly by you." is a condition that almost anyone who's not the original maintainer of a project is almost never going to meet - you would have to do a ground up rewrite of a module for this to be the case. It's not even true of any core-committer, I don't think any of us have committed > 50% of code in core and even if we had that wouldn't be an indication of knowledge because diffstat and patch complexity aren't always the same.

I don't know that the process is for changing this, but something that would actively be achievable, even if only for someone who's been on Drupal.org for years, would look something like this:

Can I apply using a project for which I am offering to become maintainer?

Yes, however you must have been consistently co-maintaining the project, active in its issue queue and committing code over a sufficient period of time and level of activity. For example you should be able to point to 10 or more significant (either impact or scope) issues that you've contributed code to and a similar number of core commits, but quality is more important than quantity here.

I also personally think that MRs should be eligible - for example if someone works on five security improvement MRs for core, does the majority of the work on those MRs, gets them reviewed and committed to core, then why do they need to apply for this role with a new module that they might not have any need for? However this is probably better as a separate issue, one thing at a time.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

🌱 Plan
Status

Active

Version

1.0

Component

Code

Created by

🇬🇧United Kingdom catch

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @catch
  • 🇺🇸United States greggles Denver, Colorado, USA

    I also personally think that MRs should be eligible

    I agree with that. I do think there's a number of issues (maybe at least 5) to show how someone behaves across different topics. And the goal should be to adjust, create, refactor at least as much code as is necessary to get a module approved. I think there was some number of lines of code like 200 that was seen as a minimal level. If someone has a MR with 200 lines of code that should qualify similarly, in my opinion.

    Thanks for raising this, catch.

    So far that page is getting edited and I'm not sure where proposals are evaluated. IMO changes are on-topic to discuss here since a main goal of the process is to ensure security education (licensing and community norms being other goals) AND because the queue is about gaining the ability to opt a project into security advisory coverage. That all means to me that security team and working group are an appropriate place to get visibility and approval of these kinds of changes.

    We should get attention of folks who work in that queue for their feedback and then be sure to wait ~2 weeks for feedback (enough time for someone deep in a project or on a long vacation to come and provide feedback).

    Also, see https://www.drupal.org/project/projectapplications/issues/3337261#commen... → for some conversation that indicates the changes catch proposes are somewhat already the practice.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    The purpose of those applications is verifying what the user who applies understands about writing secure code that follows the coding standards and correctly uses the Drupal API. Being active in the project issue queue does not let reviewers understand what the user about those topics; closing an issue because it is about a feature request a maintainer does not think it is relevant for the project does not show the user understands how to write secure code, for example.

    Some of those sentences could be "softened," as their meaning is (for example) that users who apply with a project for which they did not do most of the commits could be asked to apply using a different project, not that they will be surely asked to apply using a different project.
    The reason they have been put in a FAQ is that some users keep misunderstanding what the purpose of those applications is, and think we are going to change the project status (and therefore reward the collective effort on the project), not to allow a single user to opt projects into security advisory.

    IMO, merge requests cannot show what the users understand on writing code, especially in the case of issues where the correct change could suggested by one or more users who participate in that issue, and applied from the user who open the merge request, who could not even understand why that change has been suggested.
    Merge requests do not allow reviewers to suggest to change, for example, what is passed to the translation function/method, and see that change applied in the merge request, as that change would be off-topic for an issue that is about (for example) a different security issue, or to better describe what a method does and which parameters it accepts. Merge requests do not allow the back-and-forth the current application workflow allows, which IMO is what makes these applications helpful. (I take we are also verifying how users react to the instructions they are given, which is essentially what happens when a security issue is reported for a project and the maintainers are contacted from the Security Team.)

  • 🇺🇸United States greggles Denver, Colorado, USA

    Thanks for the feedback, apaderno.

    Part of our goal is to solve this scenario:
    * The practice of the security team is to require that a person taking over an abandoned module be able to opt it back in to security coverage. We get many people who offer to become maintainers, but they do not have that permission.
    * The process to opt in requires commit access to an existing module, or making a new one. Making a new one just to pass the test seems like something we should avoid promoting.

    So Catch and I are trying to find ways to keep the project application process as effective as it is, while making it more possible for folks to make it through without inventing a new module or becoming a maintainer of one and then doing commits on thousands of lines when a novel module would only require hundreds of lines.

    It has occurred several times now that people with thousands of lines in core and even core committers do not have the role because it is too arduous of a process. That feels like the process is not serving those people well :)

    Looking forward to hearing your thoughts. And thanks for your ongoing work in this area.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    It would be possible to make an exception for those two cases.

    For example, it would not be necessary to require that a person who is offering to become maintainer of an abandoned project is already able to opt projects into security coverage; for really abandoned projects, the offer could be accepted by any person and the vetted role granted before adding the user as maintainer / project owner. I take a project with a new maintainer is better than an abandoned project.
    People who are core committers should get the vetted role without an application. To be core committers should be an "higher" level than to be a person who just created an account on drupal.org and accepted the Git access agreement.

    As final note for this comment, I always thought that the application to be able to opt projects into security coverage were done to avoid that the Security Team would deal with people who had no clue on how to write secure code and follow the Drupal coding standards; at the same time, it avoided that users found projects that did not correctly use the Drupal API. Actually, that is the thought I had since I was handling the applications to get CVS access.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Speaking of applications to get CVS access, at that time it was possible to get CVS access by applying to be co-maintainers/maintainers of an existing project. I do not know why that possibility was removed, but that would fit the first case, although it would not help with projects where the maintainers do not reply to issues or with projects with security issues to be fixed.

  • 🇬🇧United Kingdom catch

    IMO, merge requests cannot show what the users understand on writing code, especially in the case of issues where the correct change is suggested by one or more users who participate in that issue, and applied from the user who opened the merge request, who could not even understand why that change has been suggested.

    Some merge requests have been worked on for literally years by the same one or two people. Some people have worked on dozens or hundreds of merge requests.

    I personally was given cvs access as a result of having worked on literally hundreds of core issues, but without ever having tried to maintain or create a contrib module - until eventually I wanted to create one and didn't have the role. This was back in 2007/8 or so but it would have been impossible for me to demonstrate eligibility via the current process.

  • 🇬🇧United Kingdom catch

    Made a slight update to the issue summary on how core contributions would be eligible in the suggested text.

  • 🇬🇧United Kingdom catch

    As final note for this comment, I always thought that the application to be able to opt projects into security coverage were done to avoid that the Security Team would deal with people who had no clue on how to write secure code and follow the Drupal coding standards; at the same time, it avoided that users found projects that did not correctly use the Drupal API. Actually, that is what I thought since the times I was handling the applications to get CVS access.
    It would help to know exactly what these applications are expected to avoid.

    So I think that's the same reason now, but at that time, there was no-concept of opting in to security coverage, every single project was opting in. Also as you say co-maintainers were also added to the role (because you couldn't co-maintain at all without it), so it was really to put a brake on having lots of new low-quality projects.

    Probably when git vetted changed to only applied to opting in projects to security coverage, not co-maintaining, that shortcut was removed as not necessary. But it feels to me like that was a mistake because it ignores the amount of churn in the maintenance of existing projects, and the disruption of having projects go completely out of support and unmaintained with outstanding security issues and then potentially being re-adopted - which is also work for the security team.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    I cannot say why reviewing code was deemed necessary, but it is 14 years that code reviews are done.

    I am fine with letting co-maintainers getting the vetted role without review. I wonder:

    • Was there any reason for requiring that everybody pass through a code review?
    • Since a co-maintainers → could not be able to commit code for a project, the request to be co-maintainer should be explicit about requesting that permission. Should the new role given after verifying the co-maintainer has been given the commit permission?
    • Do we need to make an exception for all the co-maintainer requests or just for some cases? I guess that, for example, for Drupal core committers are already subject to other "checks."
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Reviewing a MR or a patch does not allow me to understand what exactly that person's skills are, mostly because changing an existing module is different from writing a module. Reviewing those would become a "review" just for the matter of saying "code has been reviewed."
    Reviewing multiple MRs/patches would make the revievers' work harder. Reviewing a project is already not simple, but at least the source is all in a repository.

  • 🇬🇧United Kingdom catch

    Imagine then a patch/MR that just changes an error message or a message shown in some occasions. Reviewing those would become a "review" just for the matter of saying "code has been reviewed."

    The issue summary says this, so that kind of MR would be ineligible:

    For example you should be able to point to 10 or more significant (either impact or scope) issues that you've contributed code to and a similar number of core commits of issues where you were a significant code contributor, but quality is more important than quantity here.

    Was there any reason for requiring that everybody pass through a code review?

    I think it's because as soon as you were a co-maintainer you could also create projects. At one point cvs access meant you could commit code to any module on Drupal.org because it was all in a single repository.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    you should be able to point to 10 or more significant (either impact or scope) issues that you've contributed code to and a similar number of core commits of issues where you were a significant code contributor

    That makes quite difficult to decide when a person has the requisites to ask for the vetted role. When is an issue more significant? When is person a significant code contributor?
    Also, asking for reviewers to check 10 or more issues to verify what described there means making the reviews quite difficult.

    I think it's because as soon as you were a co-maintainer you could also create projects.

    Truly, every account is allowed to create projects, as long as the Git username is entered and the person consented to the Git access agreement. Being a co-maintainer in a project does not give more permissions on projects.
    Without the vetted role, people cannot opt projects into security advisory policy, which essentially means they cannot select a option in the project settings.

    At one point cvs access meant you could commit code to any module on Drupal.org because it was all in a single repository.

    Commits on projects were possible only to people with the Write to VCS permission on that project. That is a permission that was always present in the project settings.

  • 🇬🇧United Kingdom catch

    @apadermo #13 is only true since security coverage was opt-in, before that, any project on Drupal.org was covered by the security team. The project application process has been in place in some form a lot longer than opt-in security coverage.

  • 🇺🇸United States greggles Denver, Colorado, USA

    From #13:

    That makes quite difficult to decide when a person has the requisites to ask for the vetted role. When is an issue more significant? When is person a significant code contributor?

    These are not new questions. The queue already has answers for these questions when it relates to a module. A 10 line module without a README is rejected and told to do more. The same can be said for MR/Patch-based reviews: "Great work so far, but this does not yet appear to be enough to grant the git vetted role. Please continue your contributions and update this issue when there is more code to review."

    re #14 - I thought that projects had to be in stable releases to have security coverage, but I'm not sure. It was also the case that there were dramatically fewer modules :)

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Yes, but I take that was done to simplify things, and to reduce the need to open an application to get the vetted status. At this point, the only way to simplify the procedure is to limit who needs code review to get the vetted status.

  • 🇺🇸United States greggles Denver, Colorado, USA

    At this point, the only way to simplify the procedure is to limit who needs code review to get the vetted status.

    I'm a little confused by that. I think another way to simplify things would be to make more options for what can qualify someone to be approved?

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    The queue already has answers for these questions when it relates to a module.

    The documentation for the security advisory coverage applications is all about reviewing a project repository, not to review patches or MRs.

    In fact, the workflow for those applications is that a reviewer points out what needs to be changed, the applicant either changes the project files as requested or points out the requested change is not correct, to which reviewers can reply with an explanation of why the requested change is correct, or recognizing the required change is effectively not correct and should not be done. Alternatively, the reviewers can point out that the changes are still not correct and explain why the new changes are still not the correct ones.
    There is a back-and-forth between reviewers which is not possible with patches and MRs which, by the fact their are given in issues, are limited in scope, while in a project review the changes that can be requested are broader.

    What should the back-and-forth be in the case of patches/MRs? Should reviewers point out that X patches/reviewers are not acceptable and wait for the applicant to give links to further patches/MRs?
    More importantly, what are the possibilities for patches/MRs, which by definition are limited in scope, to introduce security issues, show a misunderstanding in using the Drupal API, or add code that does not follow the Drupal coding standards? What are the possibilities that patches/MRs introduce a significant change in an existing project, at the point that two or more classes are written from scratch from the person who then creates an application?

    I think another way to simplify things would be to make more options for what can qualify someone to be approved?

    Adding more is not simplifying a procedure; quite the opposite, is making the procedure more complex, even in term of what the reviewers should do for reviewing applications as requested.
    Reviewers are now reviewing a project repository; with these changes, they should review a project repository, or a set of patches, or a set of MRs, but being careful to first check the contribution for those patches or MRs are significant and the person is a significant contributor.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    I am not still clear which is the target of these changes.
    The only example given in the issue summary is about a core committer, who indeed cannot write half of Drupal core code. I would also imagine that if a person is really wanted as core committer, even if that person has never created a project hosted on drupal.org, that person would be granted the vetted role even without applying for that.

    Yes, there are core committers who created CVS applications, but those have been created to become co-maintainers for contributed project, not to become core committers.

  • 🇺🇸United States greggles Denver, Colorado, USA

    A problem that is a target: the core committer folks have so much integrity they don't just go around the process to grant the status to people who are core committers. They want the process to be reasonable and work for them and for other people.

    So another target problem is that the current system provides too few paths to gain the role, which causes problems for core committers, co-maintainers, and especially helping to re-support a module or take over an unsupported module.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Core maintainers should not be asked to apply in the same way other contributors do. For them, it would be sufficient to create an issue, to make the request transparent, fixed by somebody with the right role.
    For co-maintainers, it should be sufficient to do as it was done in the past; once there is a confirmation of being accepted as co-maintainers, they open an issue in the right place, fixed by somebody with the right role.

  • 🇺🇸United States greggles Denver, Colorado, USA

    That's what we're trying to describe here - is a process for core maintainers and others so they know how/where to apply.

    I don't know what you mean by "as it was done in the past" - a comaintainer gaining comaintainer does not give them the git vetted role to be able to properly maintain multiple modules.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Yes, but what described here is to ask to core maintainers and people who want to be co-maintainers for any project to give a list of 10 or more MRs/patches.
    What I am saying is that core maintainers should even be asked about those links; it should be enough they open an issue where they state they are getting added as core maintainers (possibly confirmed by somebody else) to get the vetted role.
    For people who want to be co-maintainers, they should first ask to be co-maintainers; when they are added, they open another issue where they ask for the vetted role and provide a link for the issue where they were accepted as co-maintainer, to get the vetted role.

    When the application was to give CVS access, people who offered to be co-maintainers were not asked to attach an archive file containing a project to review, like other people.

  • 🇬🇧United Kingdom catch

    Core maintainers should not be asked to apply in the same way other contributors do. For them, it would be sufficient to create an issue, to make the request transparent, fixed by somebody with the right role.

    There are core maintainers (who need commit access to core), and core subsystem maintainers though.

    For example it's theoretically possible (and I think has happened in the past), for someone to maintain a core module for several years without Drupal.org git access, because submaintaining only requires being able to make MRs/patches and review issues. However, if someone's been say taxonomy module maintainer for five years (which even in five years, they would not have written more than 50% of the code in taxonomy module), the current system makes them ineligible to get the git vetted role.

    If it's not part of the process, then what we're effectively saying is that if you're friends with someone with permission to give you the git vetted role, you can bypass the process and get it, just with an issue for transparency. Instead of that, I would like some criteria where people can demonstrate skills and previous contributions to the project, that actually reflects the way that a lot of people interact with it (i.e. via contributions to existing projects hosted on Drupal.org which they don't already have git commit access to).

  • 🇺🇸United States greggles Denver, Colorado, USA

    Yes, but what described here is to ask to core maintainers and people who want to be co-maintainers for any project to give a list of 10 or more MRs/patches.

    This is not accurate to the proposal. They are not required to do it, it's another option they can have. And it's OK if it puts some burden on them as long as its a reasonable burden.

    What I am saying is that core maintainers should not even be asked about those links...

    I don't think we want a system where "known" people can just open an issue and do something. If someone is doing significant contribution enough to become comaintainer or core maintainer/subsystem then it will be easy for them to link to 10 significant issues they've worked on.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    However, if someone's been say taxonomy module maintainer for five years (which even in five years, they would not have written more than 50% of the code in taxonomy module), the current system makes them ineligible to get the git vetted role.

    I still do not see why somebody who has been trusted for being a core sub-maintainer for years should need to follow the same procedure that people who have never been core sub-maintainers or core committers follow.

    The described procedure would still be shorter for them, since reviewers would just look at the MRs/patches, since any change to the MR/patch will be subject to the original issue, not the application.
    At the end, the described procedure would still be favorable for specific people, but a "review" is still deemed necessary.

    Let us also say that the few people that are now core maintainers (using that term to include also the people who do not have commit access on the Drupal core repository) for which I found a CVS application have passed any code review. So, the next question would be Why should they now be required to pass a "code review" for which reviewers could not even be able to point out mistakes the applicant will then need to fix?

    They are not required to do it, it's another option they can have. And it's OK if it puts some burden on them as long as its a reasonable burden.

    This issue has not been opened on the basis that MRs/patches to get the vetted role will be helpful to very few people. Quite the opposite, it has been opened because reviewing MRs/patches will be helpful for many people. I do not think we are talking about ten people who would use MRs/patches to get the vetted role.

    The burden is not put only on them, but also on reviewers.
    Talking about them, the workflow to review patches/MRs has not been yet described.

    I don't think we want a system where "known" people can just open an issue and do something.

    To get a new role on drupal.org, the procedure is to open an issue in the correct queue. (It is irrelevant which that issue queue actually is, nor if that issue queue changed over the time.) The system where "known" people can just open an issue and do something has been used to give new roles to all the people with an account on drupal.org, with the only exception where a specific role was automatically given. Yes, the person who ask for the new permission is not the person who assign the new permission, but that is obviously what I would expect also in the case of the vetted role, which is just another drupal.org role.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Anyway, just to get back on focus, what is the workflow for people who apply to get the vetted role using MRs/patches? What are reviewers expected to do?

    Bear in mind that the current application documentation is describing what reviewers do when reviewing a project. What described there does not apply for MRs/patches.

  • 🇬🇧United Kingdom catch

    I still do not see why somebody who has been trusted for being a core sub-maintainer for years should need to follow the same procedure that people who have never been core sub-maintainers or core committers follow.

    Right nor do I, that's why I opened this issue. Right now, there is no documented procedure for this, this issue is for adding one.

    However, I think there is also a wider group of people who have consistently been contributing to core (but aren't in MAINTAINERS.txt) or acting as a co-maintainer on a module for months or years, who should also be able to follow a much shorter procedure, and it would be good if the wording could encompass those too.

    i.e. if someone has worked on 10 MRs that were committed to core, then those 10 MRs passed core code review, which is not (or should not be) less stringent than the git vetted applications review - so anything that's already committed could be considered pre-reviewed, the only review might be to confirm they actually worked on them.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Right nor do I, that's why I opened this issue. Right now, there is no documented procedure for this, this issue is for adding one.

    That should be in the documentation about Drupal core and its governance. It is not documentation you find in Apply for permission to opt into security advisory coverage (which is the documentation guide the issue summary quotes), in the same way Offering to become a project owner, maintainer, or co-maintainer → does not apply to Drupal core. (I hope nobody really thinks that to be added as core maintainer is sufficient to follow that guide.)

    so anything that's already committed could be considered pre-reviewed, the only review might be to confirm they actually worked on them.

    That is not reviewing; it is checking that person really worked on them. I would not call it review just to be able to say They passed a review.
    Leaving out that part, more significant issue and significant code contributor are vague terms for which different people would give a different valuation. I would rather not have discussions on the But my issue was significant than that other issue.
    More importantly, if I just had to check the MRs/patches are effectively done by the applicant, I would take the same time I take to verify a person has been accepted as co-maintainer and give the vetted role on that basis.

  • 🇬🇧United Kingdom catch

    That should be in the documentation about Drupal core and its governance. It is not documentation you find in Apply for permission to opt into security advisory coverage

    No, because if someone who is already a Drupal subsystem maintainer or long term core contributor, wants git vetted access so they can maintain a contrib module, that is not Drupal core governance but contrib governance. What we're talking about is an easier process for people to be able to maintain contrib modules, who are known and trusted via contributions elsewhere, and one that can be documented in the place you'd expect to look to find out how to get that access.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    No, because if someone who is already a Drupal subsystem maintainer or long term core contributor, wants git vetted access so they can maintain a contrib module, that is not Drupal core governance but contrib governance.

    For the first case we are back to If they are trusted enough to be core maintainers, why should not they be trusted when they contribute with/to a project? It is not that, for example, since they have been core maintainers for years, they are automatically trusted to be drupal.org site maintainers or, vice versa, that site moderators for years are automatically trusted to be core maintainers.

    To me, it is like the case of a member of the Drupal Community Working Group that would need the site moderator role for what he does as CWG member. That person should not asked to first respond to issues in the Drupal.org site moderators issue queue, provide guidance to people submitting them, and show to be helpful and knowledgeable about the way drupal.org is maintained as Drupal.org site moderator → says. That person is trusted enough to be accepted as CWG member; he should be trusted enough to be a site moderator, which is what essentially does in the role of CWG member, with the difference that his actions are caused by a CWG report instead of an issue in the site moderators queue.

    With this I am not saying that no issue must be created. An issue is still created (for transparency), but it would be handled considering the role that person already has.

    Why do I use the CWG member example? Because there is not a drupal.org role that is automatically given and specifically to CWG members, but CWG members could need a specific drupal.org role, for example because they need to block users.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    If the aim is to simplify how the vetted role is given, instead of code review, the applicant could be asked some questions about writing secure code that correctly uses the Drupal API. In this way, applicants they would not need to write a project, or a project branch to be given that role. Yes, a question could be about code, for example Is the following code secure? Is it correctly handling the user input? followed by code taken by the applicant's module where user input is sanitized when it is written in a database (Yes, there are still issues on that approach.)

    The code review has also the purpose of teaching to the applicants, and at the same time letting more people be involved. Checking that a set of patches have been effectively done by the applicant, and that the applicant is effectively a significant contributor, is not a moment for teaching or sharing knowledge.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Just as a side note, for somebody who created a small project, I asked to review three different applications. IMO, that person has been able to make good points on how to write better code.
    I find that better than "reviewing" MRs/patches, but it works only when most people needs to pass a code review.

  • 🇬🇧United Kingdom catch

    With this I am not saying that no issue must be created. An issue should still be created (for transparency), but it would be handled considering the role that person already has.

    So would you accept something like:

    'If you are already a core subsystem or topic maintainer, you should still open an issue but just add documentation of your Drupal core role'?

    But this does not help someone who has been contributing to core for years and is not listed in MAINTAINERS.txt - there are dozens or hundreds of people like that, and they would need something closer to what is in the issue summary already.

    Checking that a set of patches have been effectively done by the applicant, and that the applicant is effectively a significant contributor, is not a moment for teaching or sharing knowledge.

    No but we're talking about people who have already got years of Drupal contribution under their belt who don't have this role yet - this entire issue is trying to find a process that works for those people - and historically there has been a lot of them - I didn't have cvs access until I'd been contributing to core for several years because I just did not need it. Then someone wanted to make me a co-maintainer of a module, couldn't do so, then I was given it by someone later that day. If I'd needed to go through the current process, it could have been several years longer.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    this entire issue is trying to find a process that works for those people

    There is no need to find a new process; it should be sufficient to grant an exception for some people. As long as the process does not include reviewing something that cannot be reviewed like a project with commits from a single person, exceptions are good for me.
    Clearly, it is not me who can decide exceptions.

  • 🇺🇸United States greggles Denver, Colorado, USA

    Maybe we're caught on word choice then?

    It seems like avpaderno you would like to see a main process and a set of allowable exceptions, while I think catch and I are advocating for the main process to include several options (one is the current process, one is having a history of significant contributions to major projects including core).

    I think it's fine to have some people reviewing and approving applications under the main process (avpaderno, sounds like you are happy to keep doing that) while others (e.g. catch) are interested in reviewing and approving applications under the exceptions.

    I think it makes sense whether main process or exceptions for their to always be a documented issue for the change in roles.

  • 🇬🇧United Kingdom catch

    For the record I don't have access to change user roles on Drupal.org so I am unable to give people the role under any process, exceptions or not.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    Actually, this issue focuses on a change on a single point of Apply for permission to opt into security advisory coverage / FAQ → (Can I apply using a project for which I am offering to become maintainer?). It does not say in which way other documentation pages should be changed, or (better) how the full policy for giving the vetted role should be changed.

    It is not a matter of choice of words, but to agree on the changes to the full policy.
    The actual policy says that a project needs to be reviewed, and the proposed changes must either be reflected on the project files or discussed because they are wrong changes. A review for a MR, or a set or MRs changes that workflow; that means the policy must be changed to describe what exactly must be done in those cases because there is no way to suggest changes that should be applied to project files, as a MR cannot change files outside the scope of the issue created for that MR. (I will leave out the fact an issue already as their own reviewers. Adding another layer of reviewers over them would not probably work well.)

    Other comments in this issue says the actual policy is a problem for Drupal core maintainers/sub-maintainers. People appointed as Drupal core maintainers (or sub-maintainers) already do not follow what reported in Offering to become a project owner, maintainer, or co-maintainer → , so I do not see why they should follow what reported in Apply for permission to opt into security advisory coverage → , except in the case Apply for permission to opt into security advisory coverage makes an exception for Drupal core maintainers. (Asking to "review" a MR, to me, is already an exception. The difference is that with a MR people who do reviews would just look at a MR changes without the possibility to give feedback.)
    The question is then: Why should people who are going to be appointed Drupal core maintainers be considered at the same level of people who still do not have the vetted role, and who probably wrote a module for their first time?

  • 🇬🇧United Kingdom catch

    A review for a MR, or a set or MRs changes that workflow; that means the policy must be changed to describe what exactly must be done in those cases because there is no way to suggest changes that should be applied to project files

    The issue summary is not suggesting that someone should manually review a core MR for code quality - in the case of an MR that has been committed, this has already happened by other core contributors and committers anyway.

    What is suggeste is that people should be able to post an issue asking for the git vetted role, pointing to a history of meaningful contribution to core (or high usage contrib modules) via MRs and issues, in lieu of a module review.

    If we made a comparison to a job interview, then the module review is like a technical interview, and the list of issues/MRS or link to MAINTAINERS.txt is more like a resumé or CV.

    The most review I would expect the process to involve is that the person actually did contribute to the issues they said they did and hasn't just linked to some random issues, clicking some links and ctrl-F to see some comments/commits happened and are somewhat substantive would be enough.

    People appointed as Drupal core maintainers (or sub-maintainers) already do not follow what reported in Offering to become a project owner, maintainer, or co-maintainer,

    There is a clearly documented process for becoming a core subsystem maintainer here: https://www.drupal.org/community/contributor-guide/role/drupal-core-subs... →

    so I do not see why they should follow what reported in Apply for permission to opt into security advisory coverage, except in the case Apply for permission to opt into security advisory coverage makes an exception for Drupal core maintainers. (

    Permission to opt into security advisory coverage only has one process, which is module review, unless you bypass that process. We are trying to add that process (or exception if you want to call it that) here. Core subsystem maintainer is a different role to module co-maintainer with its own set of responsibilities and expectations. The git vetted role is... the git vetted role.

    However I would also reiterate that core committers/subsystem maintainers are an example of the kind of people who are not well served by the current policy, they are not the only people that the policy change is intended to help. For example I was contributing to core for years over hundreds of issues before I became a subsystem maintainer (or got what was then cvs access), and there are similar people who do that now who aren't in MAINTAINERS.txt too - however it would have been easy for me to link to a few issues like http://drupal.org/node/6162 that I'd been working on.

    The question is then: Why should people who are going to be appointed Drupal core maintainers be considered at the same level of people who still do not have the vetted role, and who probably wrote a module for their first time?

    That is the current status quo, and what we are trying to change here.

Production build 0.71.5 2024