| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hmm, as much as I'd like getting rid of the old code, perhaps we shouldn't suddenly enable the new way to do things in production the moment the new CLI release timed to coincide with GitHub Universe becomes available ... |
Sorry, something went wrong.
|
I would like the switch to be made separately to a CLI release so any issues are attributed to the correct cause more easily. |
Sorry, something went wrong.
|
Both sensible points. How about we merge this after the next CLI release and do an Action release shortly after (and after GitHub Universe)? That way we'll have two versions of the Action using the same CLI but with and without the new style of tracing. |
Sorry, something went wrong.
|
Thanks for the review! Per the discussion above, I will hold off on merging until after GitHub Universe. |
Sorry, something went wrong.
|
Do we now have an integration test for this that tests that unsetting the entire environment doesn't break extraction of C++ or C#? |
Sorry, something went wrong.
There was a problem hiding this comment.
Leaving a blocking review until the Go special casing discussed on slack has been resolved.
Sorry, something went wrong.
This is a good point. There's a test in semmle-code on the indirect build tracing feature that the Action is using, but it's probably worth having an end-to-end integration test with the Action here. I have added this unsetting to one of our existing integration tests.
Thanks. I believe this has now been resolved with no changes needed as we have decided we will not special case Go going forward. |
Sorry, something went wrong.
There was a problem hiding this comment.
One minor nitpick then, LGTM otherwise
Sorry, something went wrong.
|
Aargh, looks like something is still broken 😠 Let me investigate what is going on... |
Sorry, something went wrong.
The root cause of this was https://github.com/github/codeql-coreql-team/issues/1719. I don't think this needs to block releasing this as things were already broken there. If there's no objections, I'll merge this and do a separate codeql-action release tomorrow per @alexet's suggestion so we can pinpoint any issues that may arise to this PR. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
In #766, I temporarily disabled using the new style of tracing introduced by #744 as some issues were identified with it.
These issues have now been fixed by myself and @alexet in the internal PRs https://github.com/github/semmle-code/pull/40519 and https://github.com/github/semmle-code/pull/40515, both of which will be included in the upcoming 2.7.0 release of the CodeQL CLI.
Therefore, it should be safe for the codeql-action to use indirect and multi-language tracing starting with that release. This PR sets the version flag accordingly.
cc. @hmakholm and @jbj as they reviewed the internal PR.
Merge / deployment checklist