Skip to content
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

Do not add tracking code before "var" in "for(var x of xs)" #471

Merged
merged 2 commits into from Oct 27, 2015

Conversation

@HCanber
Copy link
Contributor

HCanber commented Oct 26, 2015

Fixes #470

Do not add tracking code before var in for(var x of xs)
This occurs when this is true:

node.type == "VariableDeclaration"
node.parent.type == "ForOfStatement"

This PR includes new code in partial.js (as this was where I found for(var x in xs)) which is used in a test in coverage.js.
I hope this was the correct place to add it.

HCanber added 2 commits Oct 26, 2015
I got numbers in test/coverage.js (percent, sloc & hits) by first adding
another for-in loop into partial.js below the existing one:
    for (var o in j) {
        ++l;
    }

When the test worked and 100% code coverage was achieved, the loop was
changed to for (var o of j)
This occurs when this is true:
node.type == "VariableDeclaration"
node.parent.type == "ForOfStatement"

Fixes issue #470
@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Oct 26, 2015

LGTM. As for CI, maybe next lab should be node 4+.

@geek

This comment has been minimized.

Copy link
Member

geek commented Oct 26, 2015

@Marsup I think you are onto a good plan. I will work on ironing out the details and see about making lab 7 be node 4+

@geek geek added the bug label Oct 27, 2015
@geek geek added this to the 7.0.0 milestone Oct 27, 2015
@geek geek self-assigned this Oct 27, 2015
geek added a commit that referenced this pull request Oct 27, 2015
Do not add tracking code before "var" in "for(var x of xs)"
@geek geek merged commit cb999c1 into hapijs:master Oct 27, 2015
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.