-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add GitHub Actions annotations report #2918
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice! Shiny! I like this idea very much indeed.
Two points of feedback:
- I'd suggest staying away from changing errors to warnings as that defies expectations.
- What about adding some information in the file docblock about how to activate this with GitHub Actions ?
P.S.: The build is failing as the new file hasn't been added to the PEAR package.xml
file, though that's an easy fix to make,
Error/Warning LabelI don't know what actually constitutes an error as distinct from a warning in PHP CodeSniffer. e.g., from wiki:
and
both are automatically fixable indentation problems but designated different. I searched the documentation and code but didn't find a definition. From a CI perspective, the real question is "does this require manual intervention?" which I think is communicated correctly here. So what I'm really trying to do is a PHPCBF report that shows what has been fixed and what needs to be fixed. But when I use A compromise might be to continue fixable/not-fixable as warning/error for GitHub Actions, but prefix the message with the PHPCS warning/error label. DocblockI think this would be more appropriate in the wiki. Here's an example GitHub Actions workflow, to be saved in
Package.xmlI've added this and it seems the build is passing! |
As I thought about this over the weekend, I think it does make sense to stick with the true error/warning output from PHPCS. I was thinking of it as running the report before PHPCBF so that the errors that CI was about to fix would be reported. When thinking of it as running PHPCBF then PHPCS, the traditional output makes more sense. So what I was really trying to do was a PHPCBF report. I noticed there were projects that were full actions, but thought it's more appropriate as part of the main PHPCS project. |
Hi. Is there any insufficient in this pull request? |
Just as a heads-up, while it may still be nice to have a PHPCS native report for this, it isn't actually needed to have annotations show in GitHub. The CS2PR tool, which can be automatically installed via the - name: Install PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.1'
coverage: none
tools: cs2pr
- name: Install Composer dependencies
uses: "ramsey/composer-install@v2"
- name: Run PHPCS
run: vendor/bin/phpcs --report=checkstyle -q /path/to/code | cs2pr |
I knew cs2pr, and thought it is nice to annotate PR without any other tool for less dependencies. I use cs2pr. Thanks. |
+1 to have this merged! |
@nicodemuz This repo is abandoned and the project has continued in a new repo: https://github.com/PHPCSStandards/PHP_CodeSniffer Nothing will be merged here anymore. |
@jrfnl Thank you! I have open a similar PR there: PHPCSStandards/PHP_CodeSniffer#1190 |
Adds a report type for use in GitHub Actions which annotates PRs etc with PHPCS errors.
Nomenclature is changed to use "warning" for errors that can be automatically fixed in CI and "error" for those that cannot.