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

GH Actions: fix fixer conflict check #2243

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented May 22, 2023

While checking something else, I came across a notice in the GH Actions logs showing that the fixer conflict check was failing.

Some backtracing shows that it has been failing for the past five months since PR #2132 was merged - see: https://github.com/WordPress/WordPress-Coding-Standards/actions/runs/3643130487

Unfortunately, the way the script was set up, this failure was not showing and not failing the builds. /cc @desrosj

The changes I'm proposing now will fix that.

I've also made sure that once the current fixer conflicts are fixed, the build will properly succeed again: https://github.com/jrfnl/WordPress-Coding-Standards/actions/runs/5048089688/jobs/9055836148

Next up: fix the fixer conflicts and whatever other errors have crept in since the build originally started failing....

GH Actions: temporarily ignore one test case file for the fixer conflict check

... until upstream PR squizlabs/PHP_CodeSniffer#3833 has been merged.

This will allow for the fixer conflict check to be enabled again already and to prevent new issues from being introduced.

@jrfnl
Copy link
Member Author

jrfnl commented May 22, 2023

Upstream PR squizlabs/PHP_CodeSniffer#3833 is part of the fixes needed for this.

I have another few lined up. Not finished yet though.

@jrfnl
Copy link
Member Author

jrfnl commented May 27, 2023

Upstream PR PHPCSStandards/PHPCSUtils#476, which is included in the PHPCSUtils 1.0.6 release, is part of the fixes needed for this.

@jrfnl
Copy link
Member Author

jrfnl commented May 27, 2023

PR #2244 and #2245 are also related (and needed) to fix the failing fixer conflict check.

While checking something else, I came across a notice in the GH Actions logs showing that the fixer conflict check was failing.

Some backtracing shows that it has been failing for the past five months since PR 2132 was merged - see: https://github.com/WordPress/WordPress-Coding-Standards/actions/runs/3643130487

Unfortunately, the way the script was set up, this failure was not showing and not failing the builds.

The changes I'm proposing now will fix that.

I've also made sure that once the current fixer conflicts are fixed, the build will properly succeed again: https://github.com/jrfnl/WordPress-Coding-Standards/actions/runs/5048089688/jobs/9055836148

Next up: fix the fixer conflicts and whatever other errors have crept in since the build originally started failing....
@jrfnl jrfnl force-pushed the feature/ghactions-fail-build-on-fixer-conflicts branch from 08913ba to 96b0494 Compare May 28, 2023 20:11
…ict check

... until upstream PR squizlabs/PHP_CodeSniffer 3833 has been merged.

This will allow for the fixer conflict check to be enabled again already and to prevent new issues from being introduced.
@jrfnl
Copy link
Member Author

jrfnl commented May 28, 2023

I've added a second commit to temporarily ignore one test case file until an upstream PR has been merged. At least that allows us to turn the check back on and prevent new issues from getting into the codebase unnoticed.

Marking this PR "ready for review".

@jrfnl jrfnl marked this pull request as ready for review May 28, 2023 20:21
Copy link
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

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

@jrfnl jrfnl merged commit 3f608b2 into develop Jun 8, 2023
35 checks passed
@jrfnl jrfnl deleted the feature/ghactions-fail-build-on-fixer-conflicts branch June 8, 2023 05:06
@jrfnl
Copy link
Member Author

jrfnl commented Jun 30, 2023

I've opened issue #2274 as a reminder to remove the test exclusion as soon as we can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants