search

sirbrillig / phpcs-variable-analysis

Find undefined and unused variables with the PHP Codesniffer static analysis tool.
Other
136 stars 14 forks source link

GH Actions: various updates #283

Closed jrfnl closed 2 years ago

jrfnl commented 2 years ago

GH Actions: fix use of deprecated set-output

GitHub has deprecated the use of set-output (and set-state) in favour of new environment files.

This commit updates workflows to use the new methodology.

Refs:

GH Actions: update the xmllint-problem-matcher

~~The xmllint-problem-matcher action runner has released a new version which updates it to use node 16. This gets rid of a warning which was shown in the action logs.~~

Note: I've suggested to the author to use long-running branches for the action runner instead, which would make this update redundant, but no telling if or when they'll respond to that, let alone if they will follow my suggestion.

Refs:

šŸ†• GH Actions: harden the workflow against PHPCS ruleset errors

If there is a ruleset error, the cs2pr action doesn't receive an xml report and exits with a 0 error code, even though the PHPCS run failed (though not on CS errors, but on a ruleset error).

This changes the GH Actions workflow to allow for that situation and still fail the build in that case.

šŸ‘‰šŸ» Note: this won't get rid of all warning yet as a lot of predefined action runners also use set-output, but most of those are in the process of updating and/or have released a new version already, so the other warnings should automatically disappear over the next few weeks.

jrfnl commented 2 years ago

Note: PR #282 is needed to get the build to pass.

jrfnl commented 2 years ago

I've removed the second commit - the action runner branch has become a long-running branch, so this update is no longer needed.

sirbrillig commented 2 years ago

Great! Thank you!

sirbrillig commented 2 years ago

The PHP 7.4 static analysis failure is this, for the record:

Screen Shot 2022-10-15 at 11 44 41 AM

I don't see how your changes could have caused that so I'm guessing it's just that there was an upgrade to the analyzer. If you're fine with merging, I'm fine with that.

jrfnl commented 2 years ago

I don't see how your changes could have caused that so I'm guessing it's just that there was an upgrade to the analyzer. If you're fine with merging, I'm fine with that.

Correct. That looks to be related to an update of PHPStan, not to this PR. If you rerun CI on 2.x, the error should also show, which would confirm that it is unrelated to this PR.

Aside from that, a fix for the PHPStan issue is ready in PR #282, so if you'd merge that first, I could rebase this PR after and you can see a clean build.

jrfnl commented 2 years ago

Rebased without changes to get a passing build.

jrfnl commented 2 years ago

FYI: Added a new second commit with some workflow hardening (and updated the PR description to match).

sirbrillig commented 2 years ago

šŸ‘ Thank you!