TerMef is a terminology website created by the French Economic and Financial Ministries (Mef).
This module integrates with the TerMef API to display term definitions into node contents in Drupal 8 and Drupal 9 websites.
This module integrates with the Drupal formats and editor. Once the module installed, you have to enable the Termef tag on a given format and the Termef button for CKEditor.
Project link
https://www.drupal.org/project/termef
Git instructions
git clone --branch '1.0.x' https://git.drupalcode.org/project/termef.git
PAReview
I did run phpcs locally with Drupal and DrupalPractice standards and there are no errors.
Comments
Comment #2
flaures commentedComment #3
avpadernoComment #4
avpadernoFor these applications, we need a project where, in at least the branch used for the application, most of the commits (if not all the commits) have been done from the user who applies.
The purpose of these applications is reviewing a project to understand what the user who applies understands about writing secure code that follows the Drupal coding standards and correctly uses the Drupal API. When most of the code has been written by another user, this review cannot be easily done.
Comment #5
flaures commentedComment #6
flaures commentedSorry I did write the module but switched to corporate account to apply.
Maybe I should reapply with this account to make the review easier?
Comment #7
avpadernoComment #8
avpadernoComment #9
avpadernoThe master branch isn't used in Drupal.org projects. The other branch needs to be made the default one (if it's not already) and the master branch be deleted.
Strings shown to the users must be translatable.
VERSIONis used only by Drupal core modules.The description must be in English, as it's translated and translation only works for English.
Comment #10
avpadernoComment #11
flaures commentedMany thanks for your time and your feedback!
I have released a new version https://www.drupal.org/project/termef/releases/1.0.0-beta2 where:
- VERSION has been removed from the libraries.yml file
- all strings shown to users or descriptions are in English
The master branch has been deleted and 1.0.x is now the default branch.
Comment #12
avpadernoInstead of splitting the string into 3 pieces, it should use
t()-placeholders.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 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 #14
flaures commentedI have commited a fix to use t with placeholders : https://git.drupalcode.org/project/termef/-/commit/61d26800bf51fb2ac4d09...
Comment #15
flaures commentedThanks @apaderno for the reviews and helping me improve the project.