- Issue created by @dimitriskr
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Thank you for applying!
Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review → gives some hints for a smoother review.
The important notes are the following.
- If you have not done it yet, enable GitLab CI for the project, and fix what reported from the phpcs job. This help to fix most of what reviewers would report.
- For the time this application is open, only your commits are allowed. No other people, including other maintainers/co-maintainers can make commits.
- The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application.
- Nobody else will get the permission to opt projects into security advisory policy. If there are other maintainers/co-maintainers who will to get that permission, they need to apply with a different module.
- We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.
To the reviewers
Please read How to review security advisory coverage applications → , Application workflow → , What to cover in an application review → , and Tools to use for reviews → .
The important notes are the following.
- It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
- Reviewers should show the output of a CLI tool → only once per application. The configuration used for these tools needs to be the same configuration used by GitLab CI, stored in the GitLab Templates repository.
- It may be best to have the applicant fix things before further review.
For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues → .
- 🇮🇹Italy apaderno Brescia, 🇮🇹
For these applications, we need a project where, in at least a branch, most of the commits (but preferably all the commits) have been done from the person who created the application.
The purpose of these applications is reviewing a project to understand what the person who applies understands about writing secure code which follows the Drupal coding standards and correctly uses the Drupal API, not what all the project maintainers collectively understand about those points.Do you have a project for which most of the commits have been done by you in at least a branch? It also needs to contain enough PHP code.
- 🇬🇷Greece dimitriskr
Hi @avpaderno
Yes. Branch 2.x has my commits where I added Gitlab CI tests and fixed whatever was reported, updated README.md and replaced the usage of an unsupported library with a better one. All but README.md updates are included in the module's 2.0.0-beta1 release
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Those are not most of the commits (but preferably all the commits) . If it were enough to have commits, I would not have posted my previous comment.
- 🇬🇷Greece dimitriskr
Most of my work, upon receiving maintainership, was done in this issue 📌 paragonie/easyrsa project was abandoned long time ago Needs work at this MR which includes a lot of commits, where they have been squashed into the project's branch. Does that qualify?
- 🇺🇸United States cmlara
@dimitriskr
You may want to look at 📌 [1.0.0] Accessibility tools Needs review to see if it can provide you any solid points of argument. That was an application where a project forked a 3rd party and made minimal changes to itself. It may provide you the evidence you need that you have made 'sufficient' changes to apply..
- 🇮🇹Italy apaderno Brescia, 🇮🇹
No, commits done in a merge request are counted as one, since that is the commit done when the issue fork is merged.
No, the fact somebody forked a project does not mean we do not verify how many commits have been done. That is no evidence for anything, like a person passing with the red light on the New York state is not evidence that everybody can pass with the red light. - 🇬🇷Greece dimitriskr
In links in #2 I don't see any rule applying for the amount of commits done by the user, rather than the commits done show the user understands and uses the Drupal coding standards and the API correctly, which, in my opinion, is being displayed.
Is there another page/policy that applies here, which is not shared?
- 🇮🇹Italy apaderno Brescia, 🇮🇹
In Pre-requisites for applying for the permission to opt into security advisory coverage → :
Create your project and commit code in that project or, if you are using an existing project for which you are co-maintainer/maintainer, commit code in the existing project.
In the latter case, you need to be the person who made most of the commits, but preferably all the commits, in at least one project branch, which should be the branch used for your application.You have committed code in the project used for the application (not an issue fork created to fix an issue in an existing project). Those commits must be (preferably) the only ones done in the branch used for the application or be most of the commits for that branch.
Opting into security advisory coverage → is the only documentation guide about these applications.
- 🇺🇸United States cmlara
@avpaderno For those of us who are not D.O. Privileged Users, since there has been discussion on other pages with the same concern, can you confirm which of the links in #10 are policy documents (if any) vs just general documentation?
@avpaderno I believe the point of #7 and #9 is that the D.O. Privileged Users are required by the Drupal Code of Conduct (regarding collaborative and abusive behavior) to apply gates equally to all users.
In the linked issue the D.O. Privileged User repeatedly refused to agree that the code did not demonstrate their individual skill, non-privileged user thus must assume that such little edits is acceptable and that they can expect similar treatment on other issues.
Note: I have not read the code in full, I have simply skimmed the related MR which appears to contain a similar level of work as the linked issue that was repeatedly considered 'acceptable'
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Just a note: If D.O. Privileged User is referring to a specific person, it is rude to say D.O. Privileged User instead of that person's name or username.
I answered to that question already, and I will not do that here, since it is not on-topic, together your interpretation of the linked issue.
- 🇺🇸United States cmlara
@dimitriskr
You may want to comment on 🌱 More flexible language for git vetted status for co-maintainers of existing projects Active . This application came up in relationship to that issues today.
I'm not sure where the question regarding are these specific documents are policy have been answered unless @avpaderno is referring to discussion that other documents (Project Adoption) which has been stated are not policy without firm statements regarding other documents on D.O. Perhaps the intention is to mean that no documents on D.O. are policy (in which case how can they be used as evidence to refuse a request?).
I will note the use of "Privileged User" was intentionally chosen to identify users with enhanced privileges not available to the average user, and to align with the D.O. Code of Conduct to not question the motives of specific individuals, instead questioning the process and its implementation. @avpaderno was previously made aware of the terms NIST definition on Slack.
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Documentation guides/pages for project moderators' task are not policy. Other documentation guides could be policy, but that does not include the first.
The text gave a link to an issue where a single person with "elevated privileges" commented. It is an insult to refer to that person with a generic term.
This is the last comment I leave about those topics. This issue is not about them.
- 🇬🇷Greece dimitriskr
@avpaderno I now understand.
The rules/policy/guidelines (term TBC as I see) are not favourable for my situation.
But I shouldn't need to break an issue into multiple so I can just have more commits on a project's to-be-reviewed branch, especially for a module that's already feature-complete, but abandoned until I started (co-)maintaining it. I think we can move this review into "postponed" status until I either create more commits or the docs change. I will also write in 🌱 More flexible language for git vetted status for co-maintainers of existing projects Active and offer my POV