Separate `phpcs.xml.dist` Files for Each Plugin to Isolate Text Domains by mukeshpanchal27 · Pull Request #1002 · WordPress/performance

mukeshpanchal27 · 2024-02-21T06:04:48Z

Summary

Fixes #997

Checklist

PR has either [Focus] or Infrastructure label.
PR has a [Type] label.
PR has a milestone or the no milestone label.

thelovekesh · 2024-02-21T10:16:52Z

Tests the PHPCS config with the following commands by changing the text domains, and it's working as expected:

Commands output

➜  performance git:(fix/997-separate-phpcs) ✗ build-cs/vendor/bin/phpcs                                             
......E..................................................... 60 / 69 (87%)
.........                                                    69 / 69 (100%)



FILE: /home/thelovekesh/Desktop/loki/wordpressdev/public/content/plugins/performance/modules/database/audit-autoloaded-options/can-load.php
--------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------
 15 | ERROR | Mismatched text domain. Expected 'performance-lab' or 'default' but got 'performance'. (WordPress.WP.I18n.TextDomainMismatch)
--------------------------------------------------------------------------------------------------------------------------------------------

Time: 2.3 secs; Memory: 36MB

➜  performance git:(fix/997-separate-phpcs) ✗ build-cs/vendor/bin/phpcs plugins/auto-sizes --standard=plugins/auto-sizes/phpcs.xml.dist
.E 2 / 2 (100%)



FILE: /home/thelovekesh/Desktop/loki/wordpressdev/public/content/plugins/performance/plugins/auto-sizes/hooks.php
---------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------
 29 | ERROR | Mismatched text domain. Expected 'auto-sizes' or 'default' but got 'auto-'. (WordPress.WP.I18n.TextDomainMismatch)
---------------------------------------------------------------------------------------------------------------------------------

Time: 85ms; Memory: 16MB

swissspidy · 2024-02-21T10:23:51Z

Btw I am looking into those PHP unit test failures, probably has to do with yesterday's MariaDB update.

joemcgill

I like where this is headed, though I think we will need to update the various ways that we run PHPCS via local scripts to make sure we're running the correct ruleset for each plugin. I left a suggestion, but I'm not totally sold that this is the most maintainable approach.

Co-authored-by: Joe McGill <[email protected]>

github-actions · 2024-02-26T04:57:30Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: thelovekesh <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: joemcgill <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

felixarntz

@mukeshpanchal27 This looks great to me overall. A few small points of feedback.

mukeshpanchal27 · 2024-02-27T03:44:19Z

@felixarntz @joemcgill I have addressed the feedback in the PR. It's ready for merge now. Could you please review it when you have a moment?

felixarntz

Thanks @mukeshpanchal27, LGTM!

felixarntz · 2024-02-27T19:11:52Z

+
+	<rule ref="../../phpcs.ruleset.xml"/>
+
+	<config name="text_domain" value="auto-sizes"/>


Related to my previous feedback, we could add default as an allowed text domain for all plugins, but definitely not required as long as we don't use any core strings in them.

I think we should keep this omitted unless we require core strings.

felixarntz · 2024-02-27T20:04:09Z

@joemcgill Can you please take another look at this as you previously requested changes?

joemcgill

Thanks for the updates. I think this looks good now.

mukeshpanchal27 added 2 commits February 21, 2024 11:30

Initial draft

7017279

Add end line

31c65e9

mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release Creating standalone plugins labels Feb 21, 2024

mukeshpanchal27 requested review from swissspidy, thelovekesh and westonruter February 21, 2024 06:04

mukeshpanchal27 self-assigned this Feb 21, 2024

mukeshpanchal27 requested a review from joemcgill February 21, 2024 06:05

thelovekesh reviewed Feb 21, 2024

View reviewed changes

Comment thread composer.json

Comment thread phpcs.ruleset.xml

Comment thread plugins/auto-sizes/.gitattributes Outdated

Comment thread plugins/auto-sizes/.gitignore Outdated

swissspidy reviewed Feb 21, 2024

View reviewed changes

Comment thread phpcs.xml.dist

westonruter reviewed Feb 21, 2024

View reviewed changes

Comment thread plugins/auto-sizes/phpcs.xml.dist Outdated

Comment thread phpcs.xml.dist

Comment thread plugins/auto-sizes/phpcs.xml.dist Outdated

Address review feedbacks

94550b0

joemcgill requested changes Feb 23, 2024

View reviewed changes

Comment thread phpcs.ruleset.xml Outdated

Comment thread phpcs.xml.dist Outdated

Comment thread plugins/auto-sizes/phpcs.xml.dist Outdated

Comment thread composer.json

Comment thread phpcs.xml.dist Outdated

westonruter mentioned this pull request Feb 23, 2024

Introduce plugins folder for standalone plugins #934

Closed

This was referenced Feb 24, 2024

Move published modules to standalone plugins #1011

Merged

DX improvements in new monorepo #1012

Closed

mukeshpanchal27 and others added 5 commits February 26, 2024 09:19

Merge branch 'feature/modules-to-plugins' into fix/997-separate-phpcs

6e7548b

Apply suggestions from code review

4594694

Co-authored-by: Joe McGill <[email protected]>

Fix PHPCS

e5c123b

Exclude TextDomainMismatch sniff for files

5edaedf

Added 'lint:all' command

6b03ad0

mukeshpanchal27 marked this pull request as ready for review February 26, 2024 04:57

mukeshpanchal27 requested a review from felixarntz as a code owner February 26, 2024 04:57

mukeshpanchal27 requested a review from westonruter February 26, 2024 04:57

mukeshpanchal27 requested review from joemcgill, swissspidy and thelovekesh February 26, 2024 04:57

swissspidy approved these changes Feb 26, 2024

View reviewed changes

westonruter approved these changes Feb 26, 2024

View reviewed changes

Comment thread phpcs.xml.dist Outdated

thelovekesh approved these changes Feb 26, 2024

View reviewed changes

westonruter mentioned this pull request Feb 27, 2024

Separate phpcs.xml.dist Files for Each Plugin to Isolate Text Domains #997

Closed

felixarntz requested changes Feb 27, 2024

View reviewed changes

Comment thread phpcs.xml.dist Outdated

Comment thread phpcs.xml.dist Outdated

Comment thread .gitattributes

mukeshpanchal27 added 2 commits February 27, 2024 09:05

Revert default text domain changes

4927e3c

Exclude phpcs.xml.dist file

2b0c112

mukeshpanchal27 requested a review from felixarntz February 27, 2024 03:42

felixarntz approved these changes Feb 27, 2024

View reviewed changes

Merge branch 'feature/modules-to-plugins' into fix/997-separate-phpcs

ade03d8

This was referenced Feb 27, 2024

Move Speculation Rules to new location as purely a standalone plugin #946

Merged

Add shortcuts to simplify running PHPUnit tests for standalone plugins #1017

Merged

joemcgill approved these changes Feb 27, 2024

View reviewed changes

westonruter merged commit 3b40aa4 into feature/modules-to-plugins Feb 27, 2024

westonruter deleted the fix/997-separate-phpcs branch February 27, 2024 20:21


		<rule ref="../../phpcs.ruleset.xml"/>

		<config name="text_domain" value="auto-sizes"/>

Uh oh!

Conversation

mukeshpanchal27 commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thelovekesh commented Feb 21, 2024

Uh oh!

swissspidy commented Feb 21, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joemcgill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mukeshpanchal27 commented Feb 27, 2024

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

felixarntz Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

westonruter Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

felixarntz commented Feb 27, 2024

Uh oh!

joemcgill left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mukeshpanchal27 commented Feb 21, 2024 •

edited

Loading

github-actions Bot commented Feb 26, 2024 •

edited

Loading