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

Include custom query help in analysis results #804

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

cklin
Copy link
Contributor

@cklin cklin commented Nov 2, 2021

Merge / deployment checklist

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

@cklin cklin force-pushed the cklin/include-custom-query-help branch from ffd03a1 to cf59610 Compare November 2, 2021 18:01
@cklin cklin marked this pull request as ready for review November 2, 2021 18:24
@cklin cklin requested a review from a team as a code owner November 2, 2021 18:24
@cklin cklin force-pushed the cklin/include-custom-query-help branch from cf59610 to eef9c63 Compare November 2, 2021 18:25
Copy link
Contributor

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

Code change looks good, some suggestions for change note and tests.

CHANGELOG.md Outdated
@@ -3,6 +3,7 @@
## [UNRELEASED]

- The `init` step of the Action now supports `ram` and `threads` inputs to limit resource use of CodeQL extractors. These inputs also serve as defaults to the subsequent `analyze` step, which finalizes the database and executes queries. [#738](https://github.com/github/codeql-action/pull/738)
- When used with CodeQL bundle 2.7.1 or above, the action now includes custom query help (if it exists in Markdown files that have the same paths as the query files but with `.md` extension instead of `.ql`) in analysis results. [#804](https://github.com/github/codeql-action/pull/804)
Copy link
Contributor

Choose a reason for hiding this comment

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

How's this?

Suggested change
- When used with CodeQL bundle 2.7.1 or above, the action now includes custom query help (if it exists in Markdown files that have the same paths as the query files but with `.md` extension instead of `.ql`) in analysis results. [#804](https://github.com/github/codeql-action/pull/804)
- When used with CodeQL 2.7.1 or above, the Action now includes custom query help in the analysis results uploaded to GitHub code scanning, if available. To add help text for a custom query, create a Markdown file next to the `.ql` file containing the query, using the same base name but the file extension `.md`. [#804](https://github.com/github/codeql-action/pull/804)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is much better. I applied it to the commit.

src/codeql.ts Show resolved Hide resolved
@cklin cklin force-pushed the cklin/include-custom-query-help branch 3 times, most recently from 71b4936 to 01d7f67 Compare November 3, 2021 19:05
await codeqlObject.databaseInterpretResults("", [], "", "", "", "");
t.false(
runnerConstructorStub.firstCall.args[1].includes("--sarif-add-query-help"),
"--sarif-add-query-help is present"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the failure message? Perhaps "should not be present".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestion. I updated the failure message.

await codeqlObject.databaseInterpretResults("", [], "", "", "", "");
t.true(
runnerConstructorStub.firstCall.args[1].includes("--sarif-add-query-help"),
"--sarif-add-query-help is present"
Copy link
Contributor

Choose a reason for hiding this comment

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

should be present

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestion. I updated the failure message.

@cklin cklin force-pushed the cklin/include-custom-query-help branch from 01d7f67 to 9a44540 Compare November 3, 2021 20:19
@cklin cklin enabled auto-merge November 3, 2021 20:21
@cklin cklin merged commit 8f0825e into main Nov 3, 2021
@cklin cklin deleted the cklin/include-custom-query-help branch November 3, 2021 20:33
@github-actions github-actions bot mentioned this pull request Nov 4, 2021
5 tasks
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