Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OneObjectStructurePerFile: move from Extra to Core #1802

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Sep 22, 2019

Pulling this after @GaryJones brought this up as a suggestion on Slack.
WP Core currently has 27 violations against this rule - 3 in src, the rest in tests.

All the same, it would be a good thing to move this sniff to the Core ruleset. /cc @pento

Open questions:

  • The Extra configuration downgraded this to a warning. Should this be an error or a warning now it's moving to Core ?
  • The Extra configuration also adjusted the error message. Is this still necessary or should we just use the default message from the sniff ?
Location Message
Original sniff error message Only one object structure is allowed in a file.
Message as it was in Extra Best practice suggestion: Declare only one class/interface/trait in a file.
Message proposed for Core Best practices: Declare only one class/interface/trait in a file.

To do before this can be merged:

  • Add a rule to this effect to the handbook.
  • Adjust the Core ruleset to place the sniff in the right place with the text from the handbook.

@GaryJones
Copy link
Member

I'd say leave it as an error. I think the proposed message for Core is friendlier than the default upstream.

@jrfnl jrfnl force-pushed the feature/move-one-object-per-class-to-core branch from 9ed06a8 to bc61afb Compare October 16, 2019 15:00
@jrfnl
Copy link
Member Author

jrfnl commented Oct 16, 2019

I'd say leave it as an error. I think the proposed message for Core is friendlier than the default upstream.

I've updated the PR to leave the message at an error.

This rule still needs to be added to the WP/Core CS Handbook. Any volunteers ? /cc @pento

@jrfnl jrfnl marked this pull request as ready for review October 16, 2019 15:15
@dingo-d
Copy link
Member

dingo-d commented Oct 17, 2019

I'd merge this after the handbook is updated, is this ok?

@jrfnl jrfnl removed this from the 2.2.0 milestone Nov 11, 2019
@jrfnl jrfnl force-pushed the feature/move-one-object-per-class-to-core branch from bc61afb to 6573b97 Compare May 14, 2020 00:44
@jrfnl jrfnl added this to the 3.0.0 milestone May 14, 2020
@jrfnl jrfnl force-pushed the feature/move-one-object-per-class-to-core branch from 6573b97 to 30cb490 Compare July 11, 2020 18:16
Per the discussion had on Slack about this initiated by GaryJones.
WP Core currently has 27 violations against this rule.

In `Extra` the error message was changed and downgraded to a `warning`.

Now the sniff will be added to `Core`, per this PR, the:
* Message text will still be changed a little to be clearer, but will be stricter.
* The message will be an `error` and not be downgraded to a `warning`.

To do:
- [ ] Add a rule to this effect to the handbook.
@jrfnl jrfnl force-pushed the feature/move-one-object-per-class-to-core branch from 30cb490 to 88c5608 Compare April 9, 2021 10:44
@jrfnl
Copy link
Member Author

jrfnl commented Apr 9, 2021

Rebased to have the GH Actions checks run.

dingo-d added a commit to WordPress/wpcs-docs that referenced this pull request Apr 9, 2021
This addition will allow WordPress/WordPress-Coding-Standards#1802 to be merged to the coding standards.

This will encourage having one class/interface/trait per file.
@dingo-d
Copy link
Member

dingo-d commented Apr 9, 2021

I've opened a PR for the handbook, so once that is merged we can merge this PR as well.

jrfnl pushed a commit to WordPress/wpcs-docs that referenced this pull request Apr 12, 2021
* Adding one object structure per file 

This addition will allow WordPress/WordPress-Coding-Standards#1802 to be merged to the coding standards.

This will encourage having one class/interface/trait per file.
@dingo-d dingo-d merged commit 41f5a9c into develop Apr 12, 2021
@dingo-d dingo-d deleted the feature/move-one-object-per-class-to-core branch April 12, 2021 06:28
HoneyRole added a commit to HoneyRole/WPCSDOC that referenced this pull request Nov 22, 2022
* Adding one object structure per file 

This addition will allow WordPress/WordPress-Coding-Standards#1802 to be merged to the coding standards.

This will encourage having one class/interface/trait per file.
prodev515 pushed a commit to prodev515/VMprotect that referenced this pull request Dec 15, 2023
* Adding one object structure per file 

This addition will allow WordPress/WordPress-Coding-Standards#1802 to be merged to the coding standards.

This will encourage having one class/interface/trait per file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants