Skip to content

Get database sha from result index#1205

Merged
shati-patel merged 1 commit intogithub:mainfrom
shati-patel:get-sha-from-index
Mar 15, 2022
Merged

Get database sha from result index#1205
shati-patel merged 1 commit intogithub:mainfrom
shati-patel:get-sha-from-index

Conversation

@shati-patel
Copy link
Copy Markdown
Contributor

@shati-patel shati-patel commented Mar 14, 2022

We want to be able to link from remote query results to the exact file that's being analysed (i.e. the correct GitHub URL for the repository + the exact commit SHA that the database was built from).

https://github.com/dsp-testing/qc-run2/pull/639 adds this to the result index, and this PR reads the SHA and adds it to the AnalysisSummary. (If no SHA is found, we use HEAD as a fall back)

We don't use the SHA anywhere yet, but we'll need it in the webview soon. (I "tested" the change by setting some breakpoints and checking the variables, but there's no visible change 🤷🏽 )

Checklist

N/A - internal only 🤫

  • 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.
  • [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.

@shati-patel shati-patel marked this pull request as ready for review March 14, 2022 14:58
@shati-patel shati-patel requested a review from a team as a code owner March 14, 2022 14:58
Copy link
Copy Markdown
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.

Nice one!

@shati-patel shati-patel merged commit 9d99fc5 into github:main Mar 15, 2022
@shati-patel shati-patel deleted the get-sha-from-index branch March 15, 2022 10:30
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