Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
30 Aug 2023 at 07:12 UTC
Updated:
11 Oct 2023 at 11:14 UTC
Jump to comment: Most recent
Comments
Comment #2
abu-zakham commentedComment #3
vishal.kadamThank 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.
phpcs --standard=Drupal,DrupalPracticeon the project, which alone fixes most of what reviewers would report.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.
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.
Comment #4
vishal.kadam1. Fix phpcs issues.
2. FILE: translations_connector.info.yml
package: CustomModules should avoid using that value for the package.
Comment #5
abu-zakham commentedThank you, fixed, please check
Comment #6
vishal.kadamRest looks fine to me.
Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.
Comment #7
vinaymahale commentedComment #8
vinaymahale commentedNo issues found
Changing status to RTBC!
Comment #9
avpadernoIt is not clear what the purpose of the module is, since Drupal core already allows to create translations for existing nodes.
The project page needs to make that clearer, and to make clear why this module is required over what Drupal core already provides.
Comment #10
abu-zakham commentedThe primary objective of this module is to change a created entity as a translation for another node. I currently have a website that contains content in four different languages, and I recently migrated the content from the old site to the new one. Many of the contents were created as individual nodes, but with this module, editors will have the ability to link nodes together instead of having to recreate the translations and redirects.
I will update the project page to clarify this.
Comment #11
abu-zakham commentedComment #12
avpadernoThe first parameter of
addMessage()must be a translatable string, not a concatenation of dynamic strings.Comment #13
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 Slack #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.
I thank all the reviewers.
Comment #14
avpaderno