Hi, I'd like to promote my project "Queue Watcher" (queue_watcher) to have security team support.

Description of the Queue Watcher module

The Queue Watcher module lets you define automatic checks
for specific queues regarding their overall size.

A queue might get bloated due to missing or too slow worker jobs.
With this module, you can define size limits a queue shouldn't exceed.
During each cron run, the Queue Watcher checks the sizes of the queues
and sends reports to certain E-Mail addresses
and the logging system in case of exceeded limits.

The module also adds some Drush helper commands, e.g.
$ drush queue-watcher-lookup

.. to get a list of currently existent queues with their states and for optionally sending reports to the configured recipients (mails and/or logs).

Furthermore you can see a status summary of your queue states in the status report.

Project page: https://www.drupal.org/project/queue_watcher

Checkout the project

git clone --branch 8.x-1.x https://git.drupal.org/project/queue_watcher.git
cd queue_watcher

Similar projects

Queue UI, which can be used for directly viewing queues and processing them. Combining Queue Watcher and Queue UI together can provide a toolset for analysing, processing and reporting on problematic queues.

Other project by me

Wordfilter

Review bonus

Comments

mxh created an issue. See original summary.

jeetendrakumar’s picture

Hi,
pareview.sh gives some errors:

https://pareview.sh/node/1157

PA robot’s picture

Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2853974

Project 2: https://www.drupal.org/node/2847434

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxhauptm2850237git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

mxh’s picture

Status: Needs work » Needs review

pareview.sh gives some errors

Thanks for looking at it, but I see only warnings, no errors.

mxh’s picture

Pareview.sh: The error regarding line 348 in ConfigForm.php is just not true - I have described this param.

Manav’s picture

Status: Needs review » Needs work

Hi @max
If you want to contribute this module to drupal then you have to fix all these warnings. Every module which is contributed to drupal must have follow the standard before launch. Its should not have any warning or error. PR robot is a automated testing tools which checks the standard and any other issues on submitted project. So you have to fix these warnings first then you can apply for needs review process.

mxh’s picture

If you want to contribute this module to drupal then you have to fix all these warnings.

Now I'm confused. As described in point 6.1 from the checklist:

Note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

The one reported error regarding the missing parameter description is obviously a false positive.
The reported warnings about t() are not in the right context, as the translation is just being wrapped by some formatting, like if you would wrap your text with an HTML-tag.
The other warnings suggest me using dependency injection, which doesn't really make a lot of sense at these points. I load the objects when they are really needed, and they're not always needed. Just because I could use dependency injection there, doesn't always mean I have to. That's why it's a warning and not an error.

Edit: I'll just replace all warnings, but I guess the false positive will still remain.

klausi’s picture

Category: Support request » Task
Status: Needs work » Needs review

@Manav: minor coding standard issues by automated review tools are not application blockers, please do a real manual review.

mxh’s picture

Assigned: Unassigned » mxh
Status: Needs review » Postponed

Many thanks for the feedback so far. I have detected some more issues with this project, which I'd like to solve first.
I'll set this back to needs review when I think it's ready for it (again).

mxh’s picture

Status: Postponed » Needs work

Guess this is the proper status.

mxh’s picture

Assigned: mxh » Unassigned
Status: Needs work » Needs review

I fixed the found issues. Now the Queue Watcher is database-independent. I also replaced any \Drupal:: calls inside the classes by using dependency injection.
The errors found on https://pareview.sh/node/1157 should all be false positives. The found warnings regarding t() as well. I think the usage of string concatenation should be o.k. at the reported lines.

Thanks to anyone who's taking a look at it.

mxh’s picture

Issue summary: View changes
mxh’s picture

Issue summary: View changes
mxh’s picture

Issue summary: View changes
mxh’s picture

Status: Needs review » Fixed

https://www.drupal.org/project/queue_watcher

Feel free to report problems in the corresponding issue queue. I'm trying to target these ones as soon as possible.

klausi’s picture

Status: Fixed » Needs review

I assume you still want this reviewed to get security team support?

mxh’s picture

Thanks, didn't see it was moved automatically to the security advisory coverage applications queue. Yes, let's leave this one open for a review :)

mxh’s picture

Issue summary: View changes
mxh’s picture

Issue summary: View changes
PA robot’s picture

Issue summary: View changes

Fixed the git clone URL in the issue summary for non-maintainer users.

I'm a robot and this is an automated message from Project Applications Scraper.

apaderno’s picture

Priority: Normal » Critical

To the reviewers: Please change back the priority to Normal after doing a review.

sleitner’s picture

Priority: Critical » Normal
Status: Needs review » Needs work

Automated Review

pareview details see: https://pareview.sh/pareview/https-git.drupal.org-project-queue_watcher....

Review of the 8.x-1.x branch (commit 5c4bba5):

  • Your README.md does not follow best practices (headings need to be uppercase).
  • The queue_watcher.module does not implement hook_help().
  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
  • DrupalPractice has found some issues with your code, but could be false positives.
  • 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.

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template. See pareview details
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements
Coding style & Drupal API usage
  1. (*) use StringTranslationTrait in QueueWatcher.php to replace t() function
  2. (*) Drupal Coder Sniffer results in pareview

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

sleitner’s picture

Issue summary: View changes
apaderno’s picture

Status: Needs work » Closed (won't fix)

If you are still working on this application, you should fix all known problems and set the status to Needs review. (See also the project application workflow.)
Please don't change status of this application if you aren't sure you have time to dedicate to this application, or it will be closed again as won't fix.

I am closing this application due to lack of activity.