Skip to content

Conversation

@aeisenberg
Copy link
Contributor

This flag is already being used for runQueries, so let's use it for
finalize as well.

Merge / deployment checklist

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

@aeisenberg
Copy link
Contributor Author

I have not updated the README yet. I'll handle that soon. Also, any sort of unit tests or integration tests I should be adding or updating?

@aeisenberg
Copy link
Contributor Author

Also, there's a failing Integration Testing / runner-upload-sarif. I don't understand why.

@robertbrignull
Copy link
Contributor

Also, there's a failing Integration Testing / runner-upload-sarif. I don't understand why.

That's because you opened your PR from a fork and so the actions token doesn't have write scopes. You can just ignore the failure for this PR.

Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

I see no problem with this. I trust you that passing this flag to this command makes sense.

No need to update the readme or tests for this change. If the existing integration tests pass then that should be enough.

@aeisenberg
Copy link
Contributor Author

Thanks! Will resolve conflicts and rebase.

This flag is already being used for `runQueries`, so let's use it for
finalize as well.
@aeisenberg aeisenberg force-pushed the aeisenberg/threads-finalize branch from e5961ad to dd96eda Compare November 3, 2020 16:15
@aeisenberg aeisenberg merged commit bc1ee16 into github:main Nov 3, 2020
@github-actions github-actions bot mentioned this pull request Nov 9, 2020
aofaof0907 pushed a commit to aofaof0907/codeql-action that referenced this pull request Jul 29, 2021
This flag is already being used for `runQueries`, so let's use it for
finalize as well.
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