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

Allow runtime-set in Abstract_PHP_CodeSniffer_Check class and implement it in I18n_Usage_Check #681

Merged
merged 4 commits into from
Sep 29, 2024

Conversation

ernilambar
Copy link
Member

@ernilambar ernilambar commented Sep 29, 2024

Fixes #679

  • Accepts runtime-set arguments in an array form( key value pair)

Example implementation:

return array(
  'extensions'  => 'php',
  'standard'    => 'WordPress',
  'sniffs'      => 'WordPress.WP.I18n',
  'runtime-set' => array(
    'text_domain' => 'sample-domain',
  ),
);

Edit:

  • Implement runtime-set text_domain in I18n_Usage_Check class
  • Update i18n related tests as now errors will be increased
  • Update other test files where i18n_usage check was used for the errors

@ernilambar ernilambar added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall plugin infrastructure labels Sep 29, 2024
@ernilambar ernilambar force-pushed the 679-allow-runtime-set-in-sniff branch from c0bd597 to 8f565a3 Compare September 29, 2024 10:52
@ernilambar ernilambar changed the title Allow runtime set in Abstract PHP Sniff class Allow runtime-set in Abstract_PHP_CodeSniffer_Check class Sep 29, 2024
@ernilambar ernilambar marked this pull request as ready for review September 29, 2024 10:55
Copy link

github-actions bot commented Sep 29, 2024

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: ernilambar <rabmalin@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: davidperezgar <davidperez@git.wordpress.org>

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

@swissspidy
Copy link
Member

Example implementation:

Are you gonna do that in a follow-up PR?

@ernilambar
Copy link
Member Author

Example implementation:

Are you gonna do that in a follow-up PR?

I could not find idea to get slug value in that protected function get_args(). We don't have context or result object passed to there.

@swissspidy
Copy link
Member

We can make get_args() accept a Check_Result as a param and then pass $result to get_args() here:

// Create the default arguments for PHPCS.
$defaults = $this->get_argv_defaults( $result );
// Set the check arguments for PHPCS.
$_SERVER['argv'] = $this->parse_argv( $this->get_args(), $defaults );

@ernilambar
Copy link
Member Author

@swissspidy ok. That will afect lots of files. Separate pr makes sense?

@swissspidy
Copy link
Member

Up to you :-)

Copy link
Member

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niiice!

@ernilambar ernilambar changed the title Allow runtime-set in Abstract_PHP_CodeSniffer_Check class Allow runtime-set in Abstract_PHP_CodeSniffer_Check class and implement it in I18n_Usage_Check Sep 29, 2024
@ernilambar ernilambar added this to the 1.2.0 milestone Sep 29, 2024
Copy link
Member

@davidperezgar davidperezgar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Awesome!

@swissspidy swissspidy merged commit 74de2f8 into trunk Sep 29, 2024
30 checks passed
@swissspidy swissspidy deleted the 679-allow-runtime-set-in-sniff branch September 29, 2024 19:22
@ernilambar ernilambar mentioned this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow Abstract_PHP_CodeSniffer_Check based check to add runtime-set arguments
3 participants