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

GitHub actions output format #9443

Closed
Splines opened this issue Feb 17, 2024 · 10 comments · Fixed by #9484 or #9619
Closed

GitHub actions output format #9443

Splines opened this issue Feb 17, 2024 · 10 comments · Fixed by #9484 or #9619
Assignees
Labels
Enhancement ✨ Improvement to a component

Comments

@Splines
Copy link

Splines commented Feb 17, 2024

Current problem

GitHub actions/scripts can output a special log syntax to create warning/error messages directly in the GitHub UI. This is awesome, since you will see the warnings/errors in the summary of your last run and directly in the Files changed tab in pull requests (PRs). As an example, consider this image:

RuboCop (the linter/formatter for Ruby code) has an "GitHub Actions Formatter" directly included:

Useful for GitHub Actions. Formats offenses as workflow commands to create annotations in GitHub UI.

You can use it like this:

rubocop --format github

and everything is ready to go.

Desired solution

I've seen that Pylint allows to specify the --output-format command line flag and that a new JSON2 reporter was added recently. I wonder if there is also a GitHub reporter available. I haven't found one yet after some digging around in search engines ;)

Additional context

The feature could then be used as follows:

pylint --output-format=github
@Splines Splines added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Feb 17, 2024
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 26, 2024
@Pierre-Sassoulas
Copy link
Member

Thank you for opening this issue, great suggestion !

@crazybolillo
Copy link
Contributor

If no one is working on this I can do it

@crazybolillo
Copy link
Contributor

So I was checking the code and it seems like this could be done with --msg-template, in fact some reporters are marked as deprecated and suggest using --msg-template directly instead. Would this be the same case?

@DanielNoord
Copy link
Collaborator

I think trying if this works with a template would be nice. Then we can just document that in the documentation. You could start there?

@Splines
Copy link
Author

Splines commented Mar 5, 2024

I think trying if this works with a template would be nice. Then we can just document that in the documentation.

Once it works as template string, maybe it'd still be beneficial to have something like github available as key one can pass to the --output-format (or a similiar?) flag such that users won't have to add something like

pylint --msg-template='{msg_id}:{line:3d},{column}: {obj}: {msg}' # example from the docs

and can instead simply go ahead and run

pylint --output-format=github   # or a similar cli flag

without having to know the "internals" of how the messages are to be outputted in order to be GitHub-action-consumable? But maybe that's just me being too lazy as a user.

@DanielNoord
Copy link
Collaborator

No I think that is fair. We have recently added a new json reporter. You can probably look at that PR to see what stuff you'd need to add for it to work.

I'm on mobile right now so can't easily link the PR myself, sorry!

@Pierre-Sassoulas
Copy link
Member

I also like the shortcut. Lazy users are the best users 😄
Here's hopefuly a link to the code of json reporters (other exemples are available in this directory) : https://github.com/pylint-dev/pylint/blob/main/pylint%2Freporters%2Fjson_reporter.py

crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 5, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 5, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 5, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 5, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
@crazybolillo
Copy link
Contributor

I made a dummy test with my PR changes:

https://github.com/zoftko/balba/pull/2/files

Screenshot from 2024-03-05 19-42-03

Just wondering if we should mark all annotations as error? Or make some sort of mapping between pylint message types and github annotation types.

For example Pylint has fatal, error, warning, convention, refactor and information type messages. Github has notice, warning and error annotations.

crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 5, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
@Pierre-Sassoulas
Copy link
Member

It looks really great. I think pylint fatal should break the build, error should be mapped to error, warning to warning and pylint convention and refactor to github's notice.

crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 6, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 6, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
crazybolillo added a commit to crazybolillo/pylint that referenced this issue Mar 6, 2024
A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes pylint-dev#9443.
DanielNoord pushed a commit that referenced this issue Mar 8, 2024
* Add GitHub reporter

A new `github` reporter has been added. This reporter automatically
annotates code on GitHub's UI with the messages found during linting.

Closes #9443.

* Enable github output format for CI

* Apply corrections

Used message.C directly instead of slicing again, improved feature notes
and removed unecessary output from locally run pre-commit pylint hook.
It was kept for the manual stage since that is run on Github and needs
it to annotate code on PRs.
@Splines
Copy link
Author

Splines commented Mar 9, 2024

@crazybolillo Many thanks for the quick implementation and thanks to everyone involved in the PR ❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants