Skip to content

Conversation

@felicitymay
Copy link
Contributor

👋🏻 I'm proposing this change alongside some similar changes to the documentation. We want to make sure that people are aware of the limit on the number of results that will be processed per SARIF file. This is rarely a problem for results generated by CodeQL but a few people have encountered it when running 3rd party tools that may be "noisy".

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

inputs:
sarif_file:
description: The SARIF file or directory of SARIF files to be uploaded.
description: The SARIF file or directory of SARIF files to be uploaded. Each SARIF file should contain a maximum of 1000 results, any additional results are ignored.
Copy link
Contributor

Choose a reason for hiding this comment

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

I worry saying "each SARIF file" is misleading. If you upload a directory of files then it doesn't upload them separately but instead first concatenated them into one big SARIF file and uploads that. It would be more accurate to say "A maximum of 1000 results will be processed and any additional results will be ignored."

Does that seem ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Thanks. I hadn't realized that you could have more than one file before looking at the action, so wondered if this was a way to work around the 1000 result limit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the description with Alistair's suggestion.

Co-authored-by: hubwriter <hubwriter@github.com>
@felicitymay
Copy link
Contributor Author

Many thanks for the rapid turnaround on reviews @hubwriter and @robertbrignull

@felicitymay
Copy link
Contributor Author

I don't have permission to merge this. Can I leave it in your hands @robertbrignull?

@robertbrignull robertbrignull merged commit a1bfa76 into github:main Jan 19, 2021
@felicitymay felicitymay deleted the patch-1 branch January 19, 2021 18:13
@github-actions github-actions bot mentioned this pull request Jan 25, 2021
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.

3 participants