Files: 
CommentFileSizeAuthor
#9 interdiff.txt9.38 KBmajoely
#9 core-menu-coder-review-1533252-9.patch30.43 KBmajoely
PASSED: [[SimpleTest]]: [MySQL] 64,670 pass(es).
[ View ]
#5 drupalcs.txt3.49 KBmajoely
#5 core-menu-coder-review-1533252-5.patch24.45 KBmajoely
PASSED: [[SimpleTest]]: [MySQL] 64,608 pass(es).
[ View ]

Comments

Status:Active» Postponed

Status:Postponed» Needs work
Issue tags:-Novice

The 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.

Status:Needs work» Postponed

Postponing till feature freeze. If you want to help in the meantime, please work on the blockers on the meta issue. Thanks!

Issue summary:View changes
Status:Postponed» Active

StatusFileSize
new24.45 KB
PASSED: [[SimpleTest]]: [MySQL] 64,608 pass(es).
[ View ]
new3.49 KB

Here 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,

Status:Active» Needs review

Status:Needs review» Needs work

Assigned:Unassigned» majoely

Assigned:majoely» Unassigned
Status:Needs work» Needs review
StatusFileSize
new30.43 KB
PASSED: [[SimpleTest]]: [MySQL] 64,670 pass(es).
[ View ]
new9.38 KB

Fixed 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:

700 | WARNING | Format should be "* Implements hook_foo().", "* Implements
     |         | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
     |         | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
--------------------------------------------------------------------------------

and I am still not sure I did that correctly.