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

Allow compare view to work with quick-eval #2422

Merged
merged 1 commit into from
May 16, 2023

Conversation

aeisenberg
Copy link
Contributor

The compare view typically works by matching the results sets of queries. It only allows the results sets of queries with identical names to be compared.

Manually run queries use #select as the default result set. However, quick eval queries use a different, generated, name. Therefore, these two kinds of queries cannot be compared.

This commit changes the heuristics so that if there are no identical;y named results sets, the first result set of each query that is prefixed with # is used (this is the default result set).

It also makes a slightly nicer error message if there are no comparable results sets.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [n/a] [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

The compare view typically works by matching the results sets of
queries. It only allows the results sets of queries with identical
names to be compared.

Manually run queries use `#select` as the default result set. However,
quick eval queries use a different, generated, name. Therefore, these
two kinds of queries cannot be compared.

This commit changes the heuristics so that if there are no identical;y
named results sets, the first result set of each query that is prefixed
with `#` is used (this is the default result set).

It also makes a slightly nicer error message if there are no comparable
results sets.
Copy link
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

I don't have a huge amount of context on the area but the code changes look sensible to me.

@aeisenberg
Copy link
Contributor Author

Thanks for the review. This area is not tested very well, but it's also not used very much. I think these changes are low risk.

@aeisenberg aeisenberg merged commit 1dc83c5 into main May 16, 2023
@aeisenberg aeisenberg deleted the aeisenberg/compare-view-quick-eval branch May 16, 2023 15:41
@charisk
Copy link
Contributor

charisk commented May 16, 2023

Oh sorry I only just remembered, should we include anything in the CHANGELOG?

@aeisenberg
Copy link
Contributor Author

Right...yes. Forgot that. Thanks for reminding. I'll put something up.

aeisenberg added a commit that referenced this pull request May 16, 2023
@aeisenberg aeisenberg mentioned this pull request May 16, 2023
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