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

fix(action): output checklist #81

Closed
wants to merge 1 commit into from
Closed

Conversation

sambacha
Copy link

@sambacha sambacha commented Apr 1, 2024

Slither implies that the GitHub Action supports checklist output:

Checklist (consider using https://github.com/crytic/slither-action):
  --checklist           Generate a markdown page with the detector results
  --checklist-limit CHECKLIST_LIMIT
                        Limit the number of results per detector in the markdown file
  --markdown-root MARKDOWN_ROOT
                        URL for markdown generation

This PR does not implement checklist explicitly, as you can already get it by passing in the additional arguments option.

Adding the needed definition in the entrypoint.sh script would be explicit.

Slither implies that the GitHub Action supports checklist output:


```console
Checklist (consider using https://github.com/crytic/slither-action):
  --checklist           Generate a markdown page with the detector results
  --checklist-limit CHECKLIST_LIMIT
                        Limit the number of results per detector in the markdown file
  --markdown-root MARKDOWN_ROOT
                        URL for markdown generation
```

This PR does not implement checklist explicitly, as you can already get it by passing in the additional arguments option. 

Adding the needed definition in the `entrypoint.sh` script would be explicit.
@sambacha sambacha requested a review from elopez as a code owner April 1, 2024 00:32
Copy link
Member

@elopez elopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sambacha! The action has no checklist output property, and this PR does not implement one either, so this PR is not correct as-is. The outputs section of action.yml should only have output variables -- i.e. variables that we add to $GITHUB_OUTPUT, like sarif and stdout.

We have an example of how to use the checklist functionality on our README if you're interested in using it: https://github.com/crytic/slither-action?tab=readme-ov-file#example-workflow-markdown-report

@sambacha
Copy link
Author

Hi @sambacha! The action has no checklist output property, and this PR does not implement one either, so this PR is not correct as-is. The outputs section of action.yml should only have output variables -- i.e. variables that we add to $GITHUB_OUTPUT, like sarif and stdout.

We have an example of how to use the checklist functionality on our README if you're interested in using it: crytic/slither-action#example-workflow-markdown-report

Thank you for referring to the example. I came across this issue again and fixed it here crytic/slither#2513

I actually find the SARIF output with the ToB VSCode Extension much more useful! Thank you for following up, much appreciated!

@sambacha sambacha closed this Jul 25, 2024
@sambacha sambacha deleted the patch-1 branch July 25, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants