GitHub warning annotations #432
Conversation
| @@ -92,6 +94,13 @@ def parse_coverage_args(argv): | |||
| help=MARKDOWN_REPORT_HELP, | |||
| ) | |||
|
|
|||
| parser.add_argument( | |||
| "--github-warning-annotations", | |||
There was a problem hiding this comment.
given the idea of moving to a plugin type framework, maybe this should be implemented as a top level --format flag, similar to some other programs, so that it's possible to do --format github-native or --format xyz?
There would have to be discussion then about whether multiple formats are allowed (and if so, how)... for example, would it allow --format json,markdown,github-native or multiple instances of the same flag, something like --format github-native --format markdown:foo.md (the latter case allowing specifying the format and filename together)? If it's too invasive to take over the markdown / json / etc. reports, could at least have a single flag to control all stdout / stderr reporting.
There was a problem hiding this comment.
I will say if we move to this style then we should have it support the existing formats as well as the new one.
We kinda have to keep the existing flags to avoid breaking compatibility, but if we are going to be adding more then moving to something like this makes sense
There was a problem hiding this comment.
I actually started down the path of attempting to pass something like --github-annotations=warning or --github-annotations=error.
But I saw that customizing the context that is passed to jinja template for specific template generators wasn't so easy to adapt.
There was a problem hiding this comment.
@timkrins if you wanted to have that context could that be achieved by adding some kind of context parameter to the init of BaseReportGenerator?
There was a problem hiding this comment.
Maybe - I'll try have a go later in the week
|
So at a high level this seems like a useful feature. I have a hard time thinking about new diff-cover/diff-quality features since I dont really use these tools anymore. I just maintain the project. But it looks like Github annotations introduce some level of customizability (whats a warning, whats an error, whats a message). My gut is the approach taken here "Everything is a warning" seems reasonable. However, if others start using it I suspect the desire for customization will come up shortly after. I think im happy to merge in a "everything is a warning" pr. Do yall think there will be much demand for further customization? Do you think we could get away with "look, the flag will do the basics but if you have more complex demands that config needs to be defined in some config file"? |
|
For the use case like sqlfluff, both errors and warnings would be ideal. Being able to add code suggestions is also useful, though I suspect most people will use a tool like reviewdog instead for that. |
I don't think this is the perfect PR, but more created it as a point of discussion (and for my own use, as it solves my immediate use-case).
Adds a report generator that prints GitHub style annotations to stdout.
Right now, only
warningis supported - I couldn't immediately determine how to pass a command line switch to the generator without changing the base class.diff-cover coverage.lcov --github-warning-annotationsrunning in GitHub actions