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

anton-vlasenko commented 1 year ago

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:

oandregal commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/WordPress/gutenberg/pull/56982

getdave commented 9 months ago

Some additional discussion ongoing https://github.com/WordPress/gutenberg/discussions/59786#discussioncomment-8784550

WordPress / gutenberg