This is a follow-up issue spun off from #1818016: Make Breakpoint module pass Coder Review.

Its purpose is to address and correct in core those items discovered in a manual review of the Breakpoint module that were not caught by the current Coder review report.

Follow-up issues

Queue Issue Notes
Coder module #1820034: Identify incorrect @param/@return directives in implements hook_*() docblocks
Coder module #1820082: Ensure @param array type used in function definition (as appropriate)
Coder module #1820116: Sniff for incorrect namespaced objects in docblock @ directives
Coder module #1820868: Sniff for incorrect verb tense in one-line summaries
Coder module #1821592: Sniff for missing @throws directive
Coder module #1821616: Ensure @param namespaced object type used in function definition (as appropriate)
Coder module #1821634: Create a sniff to highlight incorrect use of 'id' in docblocks
CommentFileSizeAuthor
#5 1820512-5-breakpoint.patch10.42 KBlars toomre
#1 1820512-1-breakpoint.patch10.62 KBlars toomre

Comments

lars toomre’s picture

lars toomre commented
Component: base system » breakpoint.module
StatusFileSize
new 1820512-1-breakpoint.patch10.62 KB

The attached patch addresses all of the items in #1818016-7: Make Breakpoint module pass Coder Review that were not subsequently covered in the patch for that issue.

lars toomre’s picture

lars toomre commented
Status: Active » Needs review

Grr... Need to remember to set the right status!

lars toomre’s picture

lars toomre commented

While performing a manual review with patch applied, I see that I missed adding a @throws directive to docblock for breakpoint_get_theme_media_queries(). This needs to be added in the next roll of this patch.

Status: Needs review » Needs work

The last submitted patch, 1820512-1-breakpoint.patch, failed testing.

lars toomre’s picture

lars toomre commented
Status: Needs work » Needs review
StatusFileSize
new 1820512-5-breakpoint.patch10.42 KB

The @param directive for $group_id in _breakpoint_delete_breakpoint_groups() was wrong. Changed it from array to string and removed array from function declaration. Here is a re-rolled patch for the bot.

lars toomre’s picture

lars toomre commented
Issue summary: View changes

Added the follow-up issues list.

rainbowarray’s picture

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented
Issue summary: View changes

Is this issue and the listed follow-up issues still relevant? The follow-up issues mention the 7.x-2.x version, and there's been a fair amount of mixup work on breakpoint module since this issue over a year ago.

Are these things we need to double-check to see if the fixes have been made?

attiks’s picture

attiks commented
Status: Needs review » Postponed

Postponing since the whole system of breakpoints is going to change, see #2271529: Move breakpoint settings to theme and module *.breakpoints.yml files at root level

attiks’s picture

attiks commented
Status: Postponed » Closed (won't fix)

Closing since this is no longer relevant, see #2271529: Move breakpoint settings to theme and module *.breakpoints.yml files at root level