Add GitHub reporter #9484

crazybolillo · 2024-03-05T22:45:48Z

Type of Changes

	Type
✓	✨ New feature

Description

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

Closes #9443.

codecov · 2024-03-05T22:53:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.83%. Comparing base (6a09d29) to head (2c76860).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9484   +/-   ##
=======================================
  Coverage   95.82%   95.83%           
=======================================
  Files         173      173           
  Lines       18797    18808   +11     
=======================================
+ Hits        18012    18024   +12     
+ Misses        785      784    -1

Files	Coverage Δ
pylint/reporters/text.py	`94.89% <100.00%> (+0.44%)`	⬆️

... and 1 file with indirect coverage changes

Pierre-Sassoulas

That's pretty clean. Let's also use it for ourselves :D (before fixing the 'typo' so it's easy to test ?)

crazybolillo · 2024-03-06T14:35:46Z

I have this dummy PR which I am constantly updating with the HEAD of this PR. It shows both error, warning and notice annotations being generated by pylint's output: https://github.com/zoftko/balba/pull/2/files

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

This looks almost done already! Thanks very much!

pylint/reporters/text.py

DanielNoord · 2024-03-07T07:42:10Z

.pre-commit-config.yaml

        types: [python]
-        args: ["-rn", "-sn", "--rcfile=pylintrc", "--fail-on=I"]
+        args:
+          ["-rn", "-sn", "--rcfile=pylintrc", "--fail-on=I", "--output-format=github"]


I think this doesn't do much right? As pre-commit isn't uploaded to Github.

Instead we should use this in our workflows defined in .github

Yes you are right, it is only needed for the manual stage hook which is run in the checks.yml workflow.

DanielNoord · 2024-03-07T07:43:06Z

doc/whatsnew/fragments/9443.feature

+A new `github` reporter has been added. This reporter automatically annotates code on GitHub's user interface
+with the messages found during linting. Use it with `pylint --output-format=github`.


Suggested change

A new `github` reporter has been added. This reporter automatically annotates code on GitHub's user interface

with the messages found during linting. Use it with `pylint --output-format=github`.

A new `github` reporter has been added. This reporter returns the output of `pylint` in the format that

Github can use to automatically annotate code. Use it with `pylint --output-format=github`.

Users will still need to run the linter in the right workflow so it is not fully automatic.

DanielNoord · 2024-03-07T07:43:43Z

tests/test_self.py

            TextReporter(StringIO()),
            ColorizedTextReporter(StringIO()),
            JSON2Reporter(StringIO()),
+            GithubReporter(StringIO()),


Ideally we would also test the output of this reporter, but I would be fine with not doing so for now as it is such a nice to have feature.

Yes, I was thinking about tests but since Github is the one that will annotate after all, seems like we just need to trust on Github 🤔

Pierre-Sassoulas · 2024-03-07T07:44:28Z

Changes look great, but I can't see the result either hère or on the PR you linked (maybe because I'm on mobile?).

crazybolillo · 2024-03-07T13:35:49Z

Changes look great, but I can't see the result either hère or on the PR you linked (maybe because I'm on mobile?).

I checked on the app (mobile) and I can't see the annotations either so it seems like it is not supported. There are no annotations anymore in this PR, but the dummy one I linked still has them (just checked on a PC).

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.

Pierre-Sassoulas · 2024-03-07T13:54:46Z

It's a pity but not a blocker maybe github will evolve to show them on mobile later. (And jobs will show violations like before). I will be able to finish the review in one to three weeks, sorry for that.

github-actions · 2024-03-07T14:08:53Z

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 2c76860

DanielNoord

Thanks!!

@Pierre-Sassoulas Would you be okay with me merging this? I only need to apply the correct labels I think!

Pierre-Sassoulas · 2024-03-07T19:01:55Z

Sure @DanielNoord if you can see the result and think it looks good then please do. As I said the code itself and the design are great. Love the brevity @crazybolillo, great MR

crazybolillo force-pushed the i9443-crazybolillo branch from 2dc629d to 597c9ea Compare March 5, 2024 22:57