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

linter fails after running make test-addons #5424

Closed
MylesBorins opened this issue Feb 25, 2016 · 3 comments
Closed

linter fails after running make test-addons #5424

MylesBorins opened this issue Feb 25, 2016 · 3 comments
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.

Comments

@MylesBorins
Copy link
Contributor

The git ignored artifacts created by make test-addons are not passing the linter after the upgrade to eslint v2

This is blocking #5372 landing on LTS, it is also at the very least annoying on master.

I have a naive fix by adding test/addons/0* to the .eslintignore. PR incoming.

Should the temporary artifacts of the addons test be living in the test dir?

@Trott
Copy link
Member

Trott commented Feb 25, 2016

I think adding to .eslintignore is a fine temporary workaround. I suspect those files should indeed be linted. But just to get stuff moving, yeah, add to .eslintignore and I or someone else can take a look at fixing it at the source later. Might be a good first contribution for someone, actually.

@Trott Trott added tools Issues and PRs related to the tools directory. test Issues and PRs related to the tests. labels Feb 25, 2016
@MylesBorins
Copy link
Contributor Author

@Trott if that is the case then we should keep this issue open when #5425 lands

MylesBorins pushed a commit to MylesBorins/node that referenced this issue Feb 25, 2016
The scripts generated by `make test-addons` do not pass the linter.
For now I think it is best that we ignore it as they are not tracked in the tree.

Temporary Workaround for nodejs#5424

PR-URL: nodejs#5425
Trott added a commit to Trott/io.js that referenced this issue Feb 25, 2016
@Trott
Copy link
Member

Trott commented Feb 25, 2016

Alternative (or subsequent) fix in #5427

@Trott Trott closed this as completed in 3e3d941 Feb 26, 2016
rvagg pushed a commit that referenced this issue Feb 27, 2016
PR-URL: #5427
Fixes: #5424
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg pushed a commit that referenced this issue Feb 27, 2016
PR-URL: #5427
Fixes: #5424
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 27, 2016
PR-URL: #5427
Fixes: #5424
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 1, 2016
PR-URL: #5427
Fixes: #5424
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 2, 2016
PR-URL: #5427
Fixes: #5424
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

No branches or pull requests

2 participants