Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Had a look over the module over the weekend and I found some things inside the code that might be good to look over:
- There is a typo in the info file "durin" instead of "during"
- The module contains links.menu.yml file that is setting a link under the Structure menu item - I guess this was not intended since the link already exists under the Configuration menu item
- The links.task.yml file is putting a link inside the cores configuration routes, because of that I propose to change the route link to "Ignore" since we already know this belongs to the configuration part
- I also propose to set the modules package to "Configuration" so it belongs to the same group as other configuration modules. I've checked Configuration Read-only mode module has the same
Out of the list, I guess only the typo part is the only thing that needs to be fixed and maybe the duplicate menu entries, the rest of the points are more UX and standardisation oriented. Let me know your thoughts.
Comment | File | Size | Author |
---|---|---|---|
#2 | 2845729-2.patch | 1.58 KB | jzavrl |
|
Comments
Comment #2
jzavrl CreditAttribution: jzavrl at NDP commentedHere is also a patch of proposed changes.
Comment #3
jzavrl CreditAttribution: jzavrl at NDP commentedComment #4
tlyngej CreditAttribution: tlyngej at Annertech commentedLovely work!
Marking of as RTBC and will commit soon.
Thanks a lot.
Comment #6
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #7
tlyngej CreditAttribution: tlyngej at Annertech commented