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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented
Component: base system » breakpoint.module
FileSize
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 CreditAttribution: Lars Toomre commented
Status: Active » Needs review

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

Lars Toomre’s picture

Lars Toomre CreditAttribution: 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 CreditAttribution: Lars Toomre commented
Status: Needs work » Needs review
FileSize
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 CreditAttribution: 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
CreditAttribution: RainbowArray 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 CreditAttribution: 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 CreditAttribution: 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