Skip to content

Add extractor field in base language QL packs #3209

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

Merged
merged 2 commits into from
Apr 9, 2020
Merged

Add extractor field in base language QL packs #3209

merged 2 commits into from
Apr 9, 2020

Conversation

hmakholm
Copy link
Contributor

@hmakholm hmakholm commented Apr 6, 2020

The functionality of https://git.semmle.com/Semmle/code/pull/36591 depends on extractor fields being present in the base language packs, rather than just in test packs.

They shouldn't do any harm and have been tolerated by the qlpack.yml parser since release 2.0.1 of the CodeQL CLI.

@hmakholm hmakholm requested review from a team as code owners April 6, 2020 16:54
@hmakholm hmakholm requested review from BekaValentine and removed request for a team April 6, 2020 16:54
@hmakholm
Copy link
Contributor Author

hmakholm commented Apr 6, 2020

Actually we've been holding off on adding extractor fields to the test packs, and instead depended on a semi-undocumented hack that falls back to queries.xml files for getting extractor IDs for test.

However, since I've now verified that the only CLI release that doesn't understand these extractor fields is 2.0.0 (which there's no reason to try to keep supporting), we might as well add them in the test packs too. That makes documentation easier, because it then doesn't have to explain why the examples in the public repo are different from what we recommand for third-party writers' own tests.

@p0 p0 merged commit 6737e99 into github:master Apr 9, 2020
@hmakholm hmakholm deleted the baselib-extractor branch July 3, 2020 14:00
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