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.
Part of meta-issue #1518116: [meta] Make Core pass Coder Review
Comment | File | Size | Author |
---|---|---|---|
#9 | interdiff.txt | 9.38 KB | majoely |
#9 | core-menu-coder-review-1533252-9.patch | 30.43 KB | majoely |
#5 | drupalcs.txt | 3.49 KB | majoely |
#5 | core-menu-coder-review-1533252-5.patch | 24.45 KB | majoely |
Comments
Comment #1
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedPostponed because of #1326672: Clean up API docs for menu module
Comment #2
Lars Toomre CreditAttribution: Lars Toomre commentedThe patch in #1326672-66: Clean up API docs for menu module is getting close to commit. Thus, I am turning my attention to this sub-issue of making sure that Menu module now passes Coder review. Further, I have opened #1800744: Make file can be problematic to address the missing type hinting of @param and @return directives.
I am unable to run the correct Coder review of what is in the Menu module and flagged as being an issue. Could someone do so and post here what is reported so we all have some idea of what is still required to make the Menu module compliant with D8 coding standards?
On a related note, I have created and posted patches to #1803656: Improve Tests consistency to standards in modules A-M and #1804522: Improve Tests consistency to standards in modules N-Z that address a number of issues in the D8 Test classes. It would be great in those two issues could be reviewed and committed soon, as it will reduce the number of items that will need to be corrected in each of these sub-issues.
Also, note I have removed the 'Novice' tag since interpreting the flagged items report requires some understanding of what is a false positive and what legitimately needs to be corrected.
Comment #3
TravisCarden CreditAttribution: TravisCarden commentedPostponing till feature freeze. If you want to help in the meantime, please work on the blockers on the meta issue. Thanks!
Comment #4
TravisCarden CreditAttribution: TravisCarden commentedComment #5
majoely CreditAttribution: majoely commentedHere is my patch for the menu module with the following issues:
24 | ERROR | Missing function doc comment
that are because the getInfo() and setUp() functions don't have comments. I am not sure if there is a standard comment for those that should go there because looking through the rest of the core modules the ones I looked at had the same issue.
* Implements hook_block_view_BASE_BLOCK_ID_alter() for system_menu_block().
I changed a comment to remove an error to the code above, but I think it might be refering to a calss in the system module, if so I'm not sure what the appropriate comment is.
There were a few errors about the naming convention of variables that I haven't fixed yet.
48 | WARNING | Line exceeds 80 characters; contains 83 characters
* @param \Drupal\menu_link\MenuLinkStorageControllerInterface $menu_link_storage
Finnally, is there a way of getting around this warning or is it to be left?
A list of remaining error is attached as drupalcs.txt
Cheers,
Comment #6
majoely CreditAttribution: majoely commentedComment #7
majoely CreditAttribution: majoely commentedComment #8
majoely CreditAttribution: majoely commentedComment #9
majoely CreditAttribution: majoely commentedFixed all the errors from the coder review by having a better look at the coding standards comment section.
The only one that may pop up is:
and I am still not sure I did that correctly.
Comment #10
xjmThanks for all the work here so far. See #1518116-86: [meta] Make Core pass Coder Review. This issue is postponed until the meta issue is either closed or reopened.
Comment #11
tatarbjClosing in favor of #2571965: [meta] Fix PHP coding standards in core. In this issue the coding standards will be fixed on a sniff-per-sniff basis rather than a module-per-module basis.