-
Notifications
You must be signed in to change notification settings - Fork 98
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
Sarif output #685
Comments
Hi @ben-wilson-peak ! Could you elaborate a bit more on the workflow you're thinking of, for when you'd use a SARIF output after running the action? |
Absolutely. I'd like to use it in lieu of the comment output. Personally I find the output to be a little useless. It conveys information but it's not a call for action. It doesn't need to be SARIF really, I'm happy to change the title. I'm not sure of the functional difference between SARIF with advanced security and just line highlighting. Example here - https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#using-workflow-commands-to-access-toolkit-functions Suggested fixes directly in the PR would be a killer feature but I understand adds complexity |
ignore that accidental close, somehow hit ctrl + enter 😆 |
Thanks @ben-wilson-peak . Agree that there's some usability improvements we could do with the dependency review action to more actionable and contextual to the workflow run. We'll take this into advisement for future improvements! As far as suggested fixes goes - take a look at using Dependabot to manage dependency updates |
Looking forward to it! User story wise, we'd ideally like to avoid committing a change which introduces an issue to then have another PR to update the dependency. It would be better to block a bad dep being introduced at the source and patch it there. |
Hello, it would be great if the action was able to do SARIF output to better integrate with GH Advanced Security. Doing so would make the PR comment redundant and I believe would be more idiomatic with how the CodeQL ecosystem integrates.
The text was updated successfully, but these errors were encountered: