Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
11 Jul 2022 at 06:38 UTC
Updated:
30 Jul 2022 at 21:14 UTC
Jump to comment: Most recent
Comments
Comment #2
skymen commentedComment #3
skymen commentedComment #4
avpadernoThank you for applying! Reviewers will review the project files, describing what needs to be changed.
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 smother review.
To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.
Since the project is being used for this application, for the time this application is open, only the user who created the application can commit code.
Comment #5
avpadernodrupal_get_path()has been deprecated. New modules should useExtensionPathResolver::getPath().Typed properties are introduced in PHP 7.4, Drupal 9 requires PHP 7.3; Drupal 8.8, which is supported by the module, requires a lower version.
The
@seelines must be the last one in the comment.The verb in the description needs to use the third person singular.
Serviceisn't the class name, which should be fully qualified.Comment #6
skymen commentedHi @apaderno. Thanks for reviewing the project!
I've fixed your mentioned issues and checked the code one more time with the code sniffer. I pushed changes to the dev branch.
Only for one thing I need clarification from you. It's about your last comment "Service isn't the class name, which should be fully qualified." Could you please show me what particular line I have to change and to what?
Thanks one more time for your time.
Comment #7
avpadernoI apologize, I should have quoted more code.
The class is
TgIntegrationBotbut the comment for the constructor says it constructs aServiceobject, which isn't true. It should be like the following one. (I copied from one of the comments used in Drupal core code.)Comment #8
avpadernoComment #9
avpadernoThank you for your contribution! I am going to update your account.
These are some recommended readings to help with excellent maintainership:
You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.
Comment #10
skymen commentedFixed. Thanks!