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

Change logic for finding plugin slug in CLI #367

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

ernilambar
Copy link
Member

Fixes #364

@ernilambar ernilambar force-pushed the 364-fix-cli-arguments branch from 5eca713 to 2eee538 Compare December 29, 2023 11:18
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thank you, @ernilambar, for the PR! I've provided some feedbacks. Aside from that, everything appears to be in good from my perspective.

phpstan.neon.dist Outdated Show resolved Hide resolved
includes/Checker/CLI_Runner.php Outdated Show resolved Hide resolved
@ernilambar
Copy link
Member Author

PR updated as per the review comments.

@ernilambar ernilambar force-pushed the 364-fix-cli-arguments branch from ad435fc to 39844c9 Compare January 8, 2024 06:45
@ernilambar ernilambar force-pushed the 364-fix-cli-arguments branch from 39844c9 to aa1df25 Compare January 8, 2024 06:55
@ernilambar
Copy link
Member Author

@mukeshpanchal27 After last commit, Behat test (PHP 8.0 WP Latest) is failing. Any idea?

@swissspidy
Copy link
Member

Does it pass locally for you? There might be some error/notice happening

@ernilambar ernilambar force-pushed the 364-fix-cli-arguments branch from aa1df25 to 8d3ce4d Compare January 8, 2024 08:12
@ernilambar ernilambar force-pushed the 364-fix-cli-arguments branch from 8d3ce4d to c89110c Compare January 8, 2024 08:15
@ernilambar
Copy link
Member Author

Does it pass locally for you? There might be some error/notice happening

I could not make it run Behat tests locally. Some database error occurs and tests fails even before starting. So I usually check those in GitHub Actions.

@mukeshpanchal27
Copy link
Member

@mukeshpanchal27 After last commit, Behat test (PHP 8.0 WP Latest) is failing. Any idea?

The nikic/php-parser get updated from 4.18.0 to 5.0.0 in past few hours that may be issue as they update things related to PHP versions. cc. @swissspidy

@swissspidy
Copy link
Member

@mukeshpanchal27 After last commit, Behat test (PHP 8.0 WP Latest) is failing. Any idea?

The nikic/php-parser get updated from 4.18.0 to 5.0.0 in past few hours that may be issue as they update things related to PHP versions. cc. @swissspidy

This project dos not use PHP-Parser, nor do any of the dependencies.

I could not make it run Behat tests locally. Some database error occurs and tests fails even before starting. So I usually check those in GitHub Actions.

Have you tried running composer prepare-behat-tests first to set up the database? Or run WP_CLI_TEST_DBTYPE=sqlite composer behat to test without MySQL.


From what I can see in the logs, this failure started already at https://github.com/WordPress/plugin-check/actions/runs/7443210203 (though it worked fine on the PR branch itself)

So it's not isolated to this PR.

Unfortunately I'm unable to reproduce this locally right now, so let's deal with this in a separate PR to debug in more detail. No blocker for this PR.

@mukeshpanchal27
Copy link
Member

mukeshpanchal27 commented Jan 8, 2024

Have you tried running composer prepare-behat-tests first to set up the database? Or run WP_CLI_TEST_DBTYPE=sqlite composer behat to test without MySQL.

I've tested it locally, and it seems to be working fine for me as well. Not entirely sure why the issue persists. I've opened a dedicated issue to track and investigate that. Thank you!

@mukeshpanchal27 mukeshpanchal27 merged commit cf35624 into WordPress:trunk Jan 8, 2024
24 of 25 checks passed
@ernilambar ernilambar deleted the 364-fix-cli-arguments branch January 8, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve parsing of associative arguments to accept arguments regardless of position
3 participants