-
-
Notifications
You must be signed in to change notification settings - Fork 26
Add GitHub logging output format #230
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
Add GitHub logging output format #230
Conversation
This will be used to select other output formats on stdout. If nothing is selected, the default output format is chosen. If the parameter value is not understood, an error is printed, but the default output format is still chosen.
TheAngryByrd
left a comment
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.
Looks good overall :)
The main limitation of this is that GitHub has a hard limit of 10 annotations per PR. But for organizations that keep their number of warnings low, most PRs should not introduce more than 10 new analyzer warnings.
What happens when you go over 10? Does it just stop presenting warnings? This might also be worth mentioning in the Running during CI.md file.
| |> ignore | ||
| ) | ||
|
|
||
| let msgLogger = factory.CreateLogger("") |
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.
Worth commenting we don't want to specify a name since it would interfere with the output that github is expecting.
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.
Added in 8348afd.
I have added some explanation in 3d31e00. The limit is actually 10 per annotation type. The log will always contain all analyzer results, but only the first 10 annotations of each type will appear in the rest of the GitHub UI. |
TheAngryByrd
left a comment
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.
Approving, but I'll give others time to review too.
Thanks for this!
This output format prints results in a format that is understood as code annotations by GitHub Actions. This can be used as a replacement for SARIF files for users who don't have access to GitHub Advanced Security.
3d31e00 to
1e0d381
Compare
|
Force-pushed to auto-squash fixup commits. |
This adds a new CLI parameter,
--output-format, which can be set to eitherdefaultorgithub.The
defaultoption preserves the current way to print analyzer results to stdout.The
githuboption instead prints the analyzer results in a format that can be understood as a code annotation by GitHub Actions (reference).This feature allows users without a GitHub Advanced Security subscription to get nicely formatted analyzer results in CI for private repositories.
Here are some (lightly censored) screenshots of what it looks like in a real code base.
In the "Files changed" section of a pull request:

In the workflow log view:

The main limitation of this is that GitHub has a hard limit of 10 annotations per PR. But for organizations that keep their number of warnings low, most PRs should not introduce more than 10 new analyzer warnings.
Also, GitHub annotations only have three severity levels, so I've mapped both
HintandInfointo the::noticelevel.Closes #229.