Skip to content

Conversation

@xiemaisi
Copy link

@xiemaisi xiemaisi commented Jul 1, 2019

When I removed all uses of TrackedNode, I forgot about TrackedExpr. This PR removes its remaining two uses; performance is unaffected (internal link).

I also noticed that the unpromoted-candidate queries still used TrackedNodes, which seems wrong, so the second commit gets rid of that. This caused many changes in the expected test outputs for these queries, which I'm not sure I understand. @esben-semmle, perhaps you could comment on this?

@xiemaisi xiemaisi added the JS label Jul 1, 2019
@xiemaisi xiemaisi requested a review from a team as a code owner July 1, 2019 08:12
@ghost
Copy link

ghost commented Jul 1, 2019

LGTM.

The new expected test outputs look correct.
It looks like the previous TrackedNode refactoring broke the tests, but we missed that (55394df).

@semmle-qlci semmle-qlci merged commit a4fa298 into github:master Jul 1, 2019
@xiemaisi xiemaisi deleted the js/remove-TrackedExpr branch August 11, 2019 20:00
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.

2 participants