Skip to content

Conversation

@xiemaisi
Copy link

This teaches the autobuilder about a new environment variable LGTM_INDEX_XML_MODE, which can be either disabled (the default) or all. The latter causes all .xml files under $LGTM_SRC to be extracted. Additional XML extensions can be added via the existing LGTM_INDEX_FILETYPES variable.

The names of the XML modes were chosen for consistency with Java, though unfortunately that means they don't align well with the names of the TypeScript modes. We could allow none and full as aliases for disabled and all if desired (@asger-semmle, what do you think?).

Like the C# and Java autobuilder, we perform the actual extraction by delegating to odasa index --xml. This means that currently XML extraction does not respect include, exclude and filters. It absolutely should, but it's a bit of a non-trivial change which will have to come later.

No change note in this PR, since end users aren't expected to set this environment variable directly. This feature will be documented when support for the corresponding lgtm.yml flag has been added.

@xiemaisi xiemaisi added the JS label May 28, 2019
@xiemaisi xiemaisi added this to the 1.21.0 milestone May 28, 2019
@xiemaisi xiemaisi requested a review from a team as a code owner May 28, 2019 11:40
@asger-semmle
Copy link
Contributor

We could allow none and full as aliases for disabled and all if desired

Ugh, yes that is an unfortunate inconsistency. Since Java presumably doesn't support none and full it would probably be better to do it the other way around: adding disabled/all to the typescript: option. That way there is one convention that works everywhere.

Copy link
Contributor

@asger-semmle asger-semmle left a comment

Choose a reason for hiding this comment

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

LGTM. I can add the change to the TypeScript option in a separate PR

@semmle-qlci semmle-qlci merged commit 9fb61d5 into github:master May 28, 2019
@xiemaisi xiemaisi deleted the js/index-xml branch May 28, 2019 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants