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

Fix: leading comments added from previous node (fixes #256) #257

Merged
merged 1 commit into from
Mar 14, 2016

Conversation

kaicataldo
Copy link
Member

Looks like the mootools test is failing. Not sure if this means my change is bad or the expected output from the test is incorrect now that this change has been made. Are these generated with a command somewhere?

I ran this version of my change against the current master branch to test for speed and was surprised to find no conclusive evidence of this slowing things down (most likely because my change only runs on comments and only when the node type is Program)...Some tests were a bit slower, and others were a bit faster. So doesn't seem like a huge impact on performance 👍

@nzakas
Copy link
Member

nzakas commented Mar 13, 2016

Can you add a test for this change?

Chances are that there was an error in Mootools output that this fixes. You can run node tools/update-tests.js to update, though you should revert any other changes that tool makes as it causes some errors in other tests (haven't gotten around to fixing that).

@kaicataldo
Copy link
Member Author

Yeah, of course! Tests added. Sorry, wasn't understanding how these tests were organized/generated, but I see now. Let me know if I can do anything else!

May take a look at #212 after this PR lands, since I've familiarized myself with the code and the change is probably a similar thing :)

"raw": "'bar'"
},
"kind": "init",
"trailingComments": [
Copy link
Member Author

Choose a reason for hiding this comment

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

Correctly adds trailing comment to bar property

@nzakas
Copy link
Member

nzakas commented Mar 14, 2016

Awesome, thank you!

nzakas added a commit that referenced this pull request Mar 14, 2016
Fix: leading comments added from previous node (fixes #256)
@nzakas nzakas merged commit eadc683 into eslint:master Mar 14, 2016
@kaicataldo kaicataldo deleted the fixes256 branch April 18, 2016 04:42
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.

3 participants