Bachmann1234
GitHub warning annotations
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 warning is supported - I couldn't immediately determine how to pass a command line switch to the generator without changing the base class.
Screenshot of diff-cover coverage.lcov --github-warning-annotations running in GitHub actions
|--|
||
|
|
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.
Guess who had this same idea today.... [independently] ME! I'll help out, I would like to see this feature soon.
@timkrins can you sync with main? Is this something you want to finish? Or would you like it/wouldn't mind if i took over?
@kingbuzzman awesome regarding the format arguments. I'm a bit slammed atm but I will try and extract some time to rebase!
No rebase needed, just in github click on the "resolve conflicts" button 😉
I have rebased on main, and added the annotation format - although right now it is a TODO as I am still a bit unclear on an elegant way to pass the value down to the formatter and then template context.
@timkrins any idea when you'll get around to this? I kind of need this... If you're unwilling, i'll just merge my PR
@Bachmann1234 this is Bellissimo!!! and should be [squashed] merged as soon as possible 😇
@timkrins thank you so much for your efforts and idea! This is going to be great for me, you have no idea how much code I will delete because of this! ❤️
Alrighty! Lets go ahead and merge/release this. Thank you for the PR @timkrins and thanks for the review support @kingbuzzman