Skip to content

Pass computeDefaultStrings to query server when compiling queries#694

Merged
aeisenberg merged 3 commits intogithub:mainfrom
aeisenberg:aeisenberg/computeDefaultStrings
Dec 1, 2020
Merged

Pass computeDefaultStrings to query server when compiling queries#694
aeisenberg merged 3 commits intogithub:mainfrom
aeisenberg:aeisenberg/computeDefaultStrings

Conversation

@aeisenberg
Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg commented Nov 30, 2020

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • [n/a] Issues have been created for any UI or other user-facing changes made by this pull request.
  • [n/a] @github/docs-content-dsp has been cc'd in all issues for UI or other user-facing changes made by this pull request.

Copy link
Copy Markdown
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.

Nice. I suggest adding a query integration test.

* Whether to ensure that elements that do not have a displayString
* get reported anyway. Useful for universal compilation options.
*/
computeDefaultStrings: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Future work: This makes me think we should have an internal integration test, which checks the external and internal copies of this protocol are in sync.

@adityasharad
Copy link
Copy Markdown
Contributor

Worth adding a short changelog entry too, since it's actually user-visible. Something like "Query results without a string representation were previously not computed. They will now be computed and displayed with a default placeholder string."

@aeisenberg
Copy link
Copy Markdown
Contributor Author

Test is added and and query tests are now running in CI.

## [UNRELEASED]

- Fix bug when removing databases where sometimes the source folder would not be removed from the workspace or the database files would not be removed from the workspace storage location. [#692](https://github.com/github/vscode-codeql/pull/692)
- Always pass the `computeDefaultStrings` flag to query compilation commands. This will change the output of some queries. [#694](https://github.com/github/vscode-codeql/pull/694)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I put this into an earlier comment I can't find - I'd prefer to mention what the user-facing change here is, i.e. query results with no string representation were previously omitted, and will now be displayed with placeholder text.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. I didn't quite know how to say it. This sounds good.

@aeisenberg aeisenberg force-pushed the aeisenberg/computeDefaultStrings branch from 0f7074b to 4b4a12b Compare December 1, 2020 22:20
@aeisenberg aeisenberg force-pushed the aeisenberg/computeDefaultStrings branch from 4b4a12b to ed5dc69 Compare December 1, 2020 22:23
@aeisenberg aeisenberg merged commit 3c08baf into github:main Dec 1, 2020
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