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

Migrate to octokit handling of Annotations #207

Closed

Conversation

thebeline
Copy link
Contributor

@thebeline thebeline commented Apr 5, 2021

This does a bunch... But also, not that much. In summary, it switches to using the json output of golangci-lint and handling the formatting and reporting to GitHub inside of the action. This allows for a more flexibility in how the lint issues are handled than simply passing the text output to the Action Log (which is all golangci-lint would be able to do). With this approach, the action can resolve a number of issues, and is in a better position to resolve a few more. The active changes in this PR are:

Note on Severity Levels and ignore: Failure is determined by golangci-lint exiting with a non-zero code and the Issues List containing at least one Issue with a Severity Level equal to or greater than the setting of --failure-level. This defaults to notice, and can be one of notice, warning, or failure.

Ignore is stripped from the Issues list, and therefore is neither reported to the Log, or grounds for failure (a run that reports only Issues of severity level ignore will not fail, even if golangci-lint exits with 1). It is not possible to set the failure-level to ignore either. Additionally, it is not possible to not fail on a severity level of failure.

With that in mind, this offers a few options on Conditional Failure:

  • Set the Severity of a linter to ignore, and you will not see it or fail on it.
  • Set the Severity of a linter to notice or warning, and set --failure-level to the next severity up.

Consideration: I set the default --failure-level to notice because, in the current implementation, any Lint Issue can cause a Failure, and I didn't want to change the behavior too much (that being said, handling of ignore already changes this behavior, but that seemed expected). However, it would make the most sense, semantically, to have the default --failure-level set to warning. What are your thoughts?

As you can see, if the Action has access to the full output, there is a lot that can be done fairly quickly.

With regards to Comments (#5): There is a bit of work that would need to be done to fully do this, but this sets the stage fairly nicely.

- Allows verbose reporting of Issues in Run Terminal/Log.
- Correctly maps Issue Severity to GitHub Severity
- Additionally adds support for Severity Failure levels (defaults to 'notice' [all])
- Adds suggested fixes to Annotation Raw Details
- Prepares foundation for supporting Comments
- Adds support for Multi-Line Annotations
- Adds mapping for common Issue Severities to GitHub Severities (Code climate | Checkstyle | GitHub)
- Adds handeling of 'Ignore' Issue Severity (removes from Issue List)
- Adds support for Character Range Annotations
@thebeline
Copy link
Contributor Author

I'm aware of the failing build, this has to do with the current check_run_id resolution. I will get it sorted.

Related: https://github.community/t/getting-the-run-id-of-a-run-in-github-actions/16726/11 (evidently this is not easy, and is kind of annoying...)

@ldez ldez marked this pull request as draft April 5, 2021 15:49
@thebeline thebeline force-pushed the MigratingToJsonDataHandeling branch from 62ba2f6 to 8630882 Compare April 5, 2021 16:13
@thebeline thebeline force-pushed the MigratingToJsonDataHandeling branch from 8630882 to 94f1966 Compare April 5, 2021 16:36
@thebeline thebeline closed this Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant