Skip to content

[Console][Yaml] Linter: add Github annotations format for errors #38982

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

Merged
merged 1 commit into from
Nov 20, 2020
Merged

[Console][Yaml] Linter: add Github annotations format for errors #38982

merged 1 commit into from
Nov 20, 2020

Conversation

ogizanagi
Copy link
Contributor

@ogizanagi ogizanagi commented Nov 4, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets N/A
License MIT
Doc PR TODO

Github actions can write errors and warning directly in their output, which result into annotations into the Github checks. It can even provide a filename, line & col number, allowing to display the annnotations inside the PR diff directly, at the right place.

More advanced usage of annotations can be made using the API, but regarding the linters provided in Symfony components, it seems the shortcut using output is a great way to enhance the integration with Github Actions.

This PR starts by proposing these changes in the yaml linter:

  • add the github format, which is the same as the txt one, except for errors and warning, for which we'll adapt the output to the Github annotations format.
  • remove the txt format as default, and autodetect if the script is running in a Github action context, then use github format. If it's not, we fallback to txt as before.

Once we agree on the details, we could perform the same for other linters (xliff, twig, ...)

Here is a PR using it: ogizanagi/symfony-lint-gha-demo#2

and some screenshots:

PR checks run PR checks annotations PR diff
Capture d’écran 2020-11-04 à 09 37 07 Capture d’écran 2020-11-04 à 09 37 28  Capture d’écran 2020-11-04 à 09 38 28

(tests to add)


This was inspired by PHPStan which is already auto-adapting the output according to the CI, using https://github.com/OndraM/ci-detector

@ogizanagi ogizanagi added the Yaml label Nov 4, 2020
@ogizanagi ogizanagi added this to the 5.x milestone Nov 4, 2020
Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

We should probably have tests for the annotation reporter

@ogizanagi ogizanagi requested a review from stof November 5, 2020 09:08
@chalasr chalasr added Console and removed Console labels Nov 14, 2020
@carsonbot carsonbot changed the title [Yaml] Linter: add Github annotations format for errors [Console][Yaml] Linter: add Github annotations format for errors Nov 14, 2020
@chalasr
Copy link
Member

chalasr commented Nov 20, 2020

Thank you @ogizanagi.

@chalasr chalasr merged commit 04eec8b into symfony:5.x Nov 20, 2020
@ogizanagi ogizanagi deleted the lint-yaml-gha branch November 20, 2020 09:41
@chalasr
Copy link
Member

chalasr commented Nov 20, 2020

@ogizanagi Can you take care of creating the doc issue for this? I accidentally skipped it during the merge

@ogizanagi
Copy link
Contributor Author

ogizanagi commented Nov 20, 2020

@chalasr Sure. Thanks for merging.
I also created an issue for other linters in #39122

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.

6 participants