Skip to content

Conversation

@robertbrignull
Copy link
Contributor

@robertbrignull robertbrignull commented Sep 10, 2020

Splits up the storage and running of builtin and custom queries. This enables us to time each set separately and populate the fields in the status report.

In runQueries in analyze.ts I had to get a little bit loose with the typing in order to populate the status report. The alternative would have been a massive switch to convert strings into the well-typed field names. It didn't seem worth it to me, but I'm open to being persuaded. I added a test for this method that hopefully will catch if we accidentally rename something that breaks assumptions.

Merge / deployment checklist

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

@rneatherway
Copy link
Contributor

If we would like to use the builtin results to track the performance of the CodeQL builtin queries over time how will we distinguish between e.g. security-and-quality, the defaults, or some other suite with a few disabled?

@robertbrignull
Copy link
Contributor Author

In the init action the status report contains the queries being run, so we should be able to determine from there which suites were run and therefore get consistent timing data.

@robertbrignull
Copy link
Contributor Author

Although I noticed we aren't populating the status report quite right. I've opened #219 to fix this.

@rneatherway
Copy link
Contributor

Great, I'm happy then.

@robertbrignull
Copy link
Contributor Author

Just for the record, I've confirmed that the status reports are getting through correctly and this timing data is arriving.

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.

3 participants