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

Proposal to align Gutenberg coding standards with Core by temporarily disabling the WordPress-Docs ruleset #56487

Closed
anton-vlasenko opened this issue Nov 23, 2023 · 8 comments · Fixed by #56982
Assignees
Labels
[Type] Code Quality Issues or PRs that relate to code quality [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@anton-vlasenko
Copy link
Contributor

anton-vlasenko commented Nov 23, 2023

What problem does this address?

After reviewing recent discussions, it has become apparent that there is a need to reconsider the use of the WordPress-Docs coding standard in the Gutenberg project. This proposal aims to align Gutenberg's coding standards more closely with Core by removing or temporarily disabling the WordPress-Docs ruleset.

Core currently does not use the WordPress-Docs ruleset. The absence of this ruleset in Core leads to discrepancies when code is backported from Core to Gutenberg. This lack of parity creates situations where code compatible with Core standards fails to pass Gutenberg CI checks, adding unnecessary complexity to the synchronization process.

This discrepancy has led to extra efforts in aligning the codebase between Gutenberg and Core, sometimes involving unnecessary code alterations to satisfy the linter.

Based on the discussions that have already taken place, the advantages and disadvantages of retaining the WordPress-Docs ruleset in Gutenberg can be formulated as follows:

Advantages of disabling the WordPress-Docs ruleset in Gutenberg:

  • simplifies the backporting process from Core to Gutenberg;
  • reduces contributor workload.

Advantages of having the WordPress-Docs ruleset enabled in Gutenberg:

  • ensures that the documentation (DocBlocks, inline comments, etc.) complies with stricter (than those of Core) standards.

What is your proposed solution?

Consider temporarily disabling the WordPress-Docs ruleset in Gutenberg.
This change would simplify the backporting process from Core to Gutenberg and reduce the workload associated with aligning the two projects.
The ruleset can be re-enabled once Core incorporates it.

Related discussions:

  1. GitHub workflows: parity with Core #48603 (review)
  2. HTML API: Backport updates from Core #52908
  3. https://core.trac.wordpress.org/ticket/50744
@anton-vlasenko anton-vlasenko added [Type] Code Quality Issues or PRs that relate to code quality [Type] Discussion For issues that are high-level and not yet ready to implement. labels Nov 23, 2023
@oandregal
Copy link
Member

I'd prioritize a seamless backport over maintaining WordPress-Doc ruleset enabled in Gutenberg if it comes down to that. I ran into this occasionally while backporting, see.

Related conversation: trying to force new standards in a stablished codebase can be daunting and throw innumerable errors on existing code - preventing us from enabling new standards. In the past, I worked on this issue using the following approach: only the new code news to adhere to the new standards introduced, so the old code can be migrated at a slower rhythm. To do that, I develop some code that would modify the linter report, downgrading from errors to warnings the errors that were reported in lines untouched by a given PR. Sharing in case it helps.

@anton-vlasenko
Copy link
Contributor Author

anton-vlasenko commented Dec 4, 2023

@oandregal, I really like your idea of converting these errors to warnings because it will not require disabling or removing the WordPress-Docs ruleset.
Thank you for sharing this and your npm package!
P.S. Fortunately, PHPCS can be easily configured to do just that:

<rule ref="WordPress-Docs">
    <type>warning</type>
</rule>

@hellofromtonya
Copy link
Contributor

tl;dr I agree and support this proposal 👍

WordPress Core itself does not use the WordPress-Docs ruleset. A ticket was introduced but then closed 3 years ago. With WPCS 3 released, this ruleset might be relooked at for Core sometime next year.

But for today, to achieve Gutenberg-to-Core parity, both repos need to maintain the same sniff packages. Not doing so will create problems and unnecessary busy work in porting code back and forth between the repos. The goal is to ease the burden of merging back-and-forth while keeping parity.

If and when Core itself does move forward to use the ruleset, then Gutenberg should also (re)adopt. But given the larger code base in Core, this kind of ruleset change needs careful consideration. Thus, the adoption should be coordinated to maintain parity.

@azaozz
Copy link
Contributor

azaozz commented Dec 5, 2023

This change would simplify the backporting process from Core to Gutenberg and reduce the workload

when Core itself does move forward to use the ruleset, then Gutenberg should also (re)adopt.

Sounds good. Seems this can be considered a bug, not an enhancement. Lets just fix it :)

@GaryJones
Copy link
Member

It's not just about adoption/usage in core or any other plugin; the WordPress-Docs ruleset is nowhere near complete compared to what's expected of code documentation in the handbook.

Makes sense to drop it for Gutenberg for now.

@aaronjorbin
Copy link
Member

Based on the lack of objection and multiple people, including core committers and Gutenberg core folks, I think this should be considered blessed.

@anton-vlasenko
Copy link
Contributor Author

anton-vlasenko commented Dec 11, 2023

I think this should be considered blessed.

Thank you, @aaronjorbin.
I'd like to allow a day or two for others to weigh in.
If there are no objections within a day or two, I will submit a pull request with the implementation of this proposal.

I think this should be considered blessed.

Updated (December 13th, 2023): I agree and also think this proposal should be considered blessed as there has been no additional feedback.

PR: #56982

anton-vlasenko added a commit that referenced this issue Dec 13, 2023
…#56982)

Remove the WordPress-Docs coding standard and associated changes. Refer to #56487 for more details.
@anton-vlasenko anton-vlasenko self-assigned this Dec 13, 2023
artemiomorales pushed a commit that referenced this issue Jan 4, 2024
…#56982)

Remove the WordPress-Docs coding standard and associated changes. Refer to #56487 for more details.
@getdave
Copy link
Contributor

getdave commented Mar 14, 2024

Some additional discussion ongoing #59786 (reply in thread)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Code Quality Issues or PRs that relate to code quality [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants