Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#49122 closed enhancement (fixed)

Coding Standard : Javascript - /src/js/_enqueues/wp/customize/base.js

Reported by: ankitmaru Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.3.2
Component: Customize Keywords: has-patch
Focuses: javascript, coding-standards Cc:


Coding Standard : Javascript - /src/js/_enqueues/wp/customize/base.js

Attachments (1)

base_js_coding_standard_issues.diff (4.0 KB) - added by ankitmaru 2 years ago.

Download all attachments as: .zip

Change History (5)

#1 @SergeyBiryukov
2 years ago

  • Component changed from General to Customize
  • Milestone changed from Awaiting Review to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#2 @SergeyBiryukov
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 47038:

Coding Standards: Add missing braces to if conditions in js/_enqueues/wp/customize/base.js.

Props ankitmaru.
Fixes #49122.

#3 @adamsilverstein
2 years ago

Noting that [r47038] also added an equality check change to a strict equality check, which looks good here - however in some circumstances this might break intended functionality, so its important to review these types of changes very carefully.

@ankitmaru & @SergeyBiryukov I appreciate your effort and contributions here, however I'm concerned that these changes are being made to refactor old coding to conform to coding standards for standards sake only. I'm all for coding standards, and improving our code quality, and in fact we have been working towards adding Prettier so our agreement can be enforced. I'm mainly concerned that the refactoring effort here might be wasted effort, or introduce unintended side effects. See https://make.wordpress.org/core/handbook/contribute/code-refactoring/.

If there is a desire to refactor old code to bring it up to standards, I would like to coordinate the effort with our JS team, I would be happy to bring it up at our next weekly JS chat.

This ticket was mentioned in Slack in #core-js by adamsilverstein. View the logs.

2 years ago

Note: See TracTickets for help on using tickets.