[D8] Admin CSS

Thank you for applying! I added the Git instructions for non-maintainer users and the PAReview checklist link. Reviewers will check the project and post comments to list what should be changed.

If you haven't done it, yet, please check the PAReview report and fix what needs to be fixed. There could be some false positives; verify that what reported is correct, before making any change.

There are some coding standards issues to be fixed in the editor.js, the editor.css, and the README.md files.

Git errors:

• There is still a master branch, make sure to set the correct default branch: https://www.drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in https://www.drupal.org/node/1127732

Review of the 8.x-2.x branch (commit 5b5dae8):

• The admincss.module does not implement hook_help(). See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... .
• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• ESLint has found some issues with your code (please check the JavaScript coding standards).
  

  /var/vhosts/c214000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/js/editor.js: line 35, col 56, Error - Unexpected trailing comma. (comma-dangle)
  /var/vhosts/c214000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/js/editor.js: line 71, col 9, Error - Expected space or tab after '/*' in comment. (spaced-comment)
  /var/vhosts/c214000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/js/editor.js: line 96, col 25, Error - Missing space before function parentheses. (space-before-function-paren)
  
  3 problems
  

• No automated test cases were found, did you consider writing PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script.

FILE: ...0/site1101/web/vendor/drupal/pareviewsh/pareview_temp/css/editor.css
--------------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 10 LINES
--------------------------------------------------------------------------
  2 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  3 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  4 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  5 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  6 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 10 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 11 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 12 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 13 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 14 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.md
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 23 | WARNING | Line exceeds 80 characters; contains 97 characters
--------------------------------------------------------------------------

Time: 635ms; Memory: 4Mb

Given that the plan is fixing the code to use the machine name of the project where that is used, would not waiting you change/add more code be better? The more edits you do, the better is for this application.

Hi @codebymikey,

Thank you for contribution!!!

Please make sure to change the status, after you made the fixes. Then only the reviewer can give you the feedback for any other changes.

To make it clear: These applications are for giving users the Drupal role that allows them to opt into security coverage for every project they created or they will create. With another user who made 12 commits, we cannot give that Drupal role to a single user, nor do we give that Drupal role for more than one user and for the same project.

That's why I said that the more code you commit the better is for this application.

The purpose of these applications is reviewing the code written from a user to check what that user understands about writing secure code that follows the coding standards and that correctly uses the Drupal API.
In this project, the only changes you have done are for using the project machine name where that should be used, which is fine, as that means you understand that aspect of Drupal programming, but it doesn't say much about the rest, as the code has been written by another user.

Many users misunderstand this: These applications aren't about projects, but users. We give users a Drupal role that allows them a higher participation on Drupal.org. It's similar to become webmasters on Drupal.org. Everybody can ask for that Drupal role, but they are given the role of webmasters for their activity on drupal.org, not another user's activity on drupal.org.

If these applications were for projects, we would not give any Drupal role to users, but we would change the settings for each project used in the applications. This is what we do when the code has been written from more than a user, or when the project contains few code lines, but these are exceptions to what these applications serve.

you need to flush CSS cache after saving your configuration. Because supposed I'm enabled " Aggregate CSS files". Your config CSS is not reflecting. Please check it.

The code of the module looks good! Then I looked at other drupal.org commits by codebymikey and they also look good and indicate that they have a good understanding how to write Drupal code. Even the admincss commits are quite substantial on their own. Therefore I think we can directly promote codebymikey.

Thanks for your contribution, codebymikey!

I updated your account so you can opt into security advisory coverage now.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

@kiamlaluno: hm, why did you add the "PAReview: single application approval" again? Please don't undo my approvals without giving a reason. As I said in #15: I looked at other drupal.org commits by codebymikey and they also look good and indicate that they have a good understanding how to write Drupal code. Even the admincss commits are quite substantial on their own. Therefore I think we can directly promote codebymikey.

Any reason why we should not grant codebymikey the security coverage role?

@klausi Then you should do the same with my reviews.

None of the points you mentioned change the result of an application, since:

• How many projects a user created is irrelevant, since we just review a project chosen by the user who applies
• These applications aren't for group work, since we review what a single user wrote and we give a Drupal role to that user
• To review what a single user understands about writing secure code that follows the coding standards and correctly uses the Drupal API, the code must provide a link to a project containing the code they wrote alone because it's difficult, or long, to check what a single user understands about writing Drupal code when the code has been written from more than a user; most importantly, it would make the task of reviewer harder, and we depend from the review other users do

The right question would be: Why should codebymikey get the role when users who wrote alone a project that contained less code don't get it?

The module is already set to opt into security coverage, as the project page says.

Stable releases for this project are covered by the security advisory policy.

@kiamlaluno

• The goal of this process is to evaluate if a user can write secure code and is able to contribute meaningful projects on drupal.org. codebymikey has demonstrated that and has written enough code in multiple projects to demonstrate that.
• We don't evaluate group work, we evaluate the contributions of a single user. codebymikey has made enough meaningful code contributions to demonstrate that.
• I put in the hard reviewer work to go trough codebymikey's visible contributions and found that they all add up to a meaningful understanding that allows us to give them the security coverage role.

To answer your question: codebymikey should get this role because they have enough meaningful contributions and understand how to publish a module on drupal.org. Other users that write a module alone have no disadvantage because of that, we will also approve them. In fact we will approve anybody that demonstrates their knowledge in this process.

Setting this back to RTBC to get feedback from kiamlaluno.

codebymikey already has the vetted role, since he created a new application and got it.

Ah, even better. Thank you!