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

Recursive .eslintrc lookup causing problems #399

Closed
rvagg opened this issue Apr 25, 2017 · 4 comments
Closed

Recursive .eslintrc lookup causing problems #399

rvagg opened this issue Apr 25, 2017 · 4 comments
Assignees

Comments

@rvagg
Copy link
Member

rvagg commented Apr 25, 2017

Failures arising out of nodejs/node#12498 (comment) appear to be related to an rules adjustments in nodejs/node, or upgrades in the ESLint versions in projects being tested. The failures are lint related, with rules either invalid (too new) or failing. The problem is that when a project uses ESLint to check its own code, it looks in parent directories for find an .eslintrc directory and it ends up defaulting to Node's .eslintrc.yaml file, so we're linting their code using our rules and our version of the config file.

e.g. in ./citgm_tmp/acorn I can run npm i and it fails on ESLint errors. If I rm ../../.eslintrc.yaml (the nodejs/node one) then npm i, it passes just fine.

Unfortunately the fix appears to be a little tricky. I've tried putting an empty, or even a basic .eslintrc (and .eslintrc.yaml in ../ (i.e. ./citgm_tmp from the root) but it still does the same thing. The rules appear to cascade from each available configuration source, not stopping once it finds one, so the only way around seems to be to remove our .eslintrc.yaml out of the way completely.

An ideal would be to remove the nodejs/node repo from the path completely, unfortunately since the citgm jobs in Jenkins are tied to that repo then you only get that in your root so I'm not sure there's an elegant way to achieve this. Perhaps as a first step in Jenkins we move ./* to a subdirectory? That could get messy but at least we'd get it out of the way.

@nodejs/build

@Trott
Copy link
Member

Trott commented Apr 25, 2017

I think if you create a ./citgm_tmp/.eslintrc file and put { "root": true } in it, that ought to fix things. Or something like that. /cc @not-an-aardvark

@Trott
Copy link
Member

Trott commented Apr 25, 2017

@not-an-aardvark
Copy link

I think if you create a ./citgm_tmp/.eslintrc file and put { "root": true } in it, that ought to fix things. Or something like that.

Yes, this will work. We do a similar thing in eslint-canary.

@gibfahn gibfahn self-assigned this Apr 25, 2017
gibfahn added a commit to gibfahn/citgm that referenced this issue Apr 25, 2017
Eslint searches parent directories looking for config files until it
finds one with "root: true" in it. This sets "root: true" in CitGM's
tmpDir to avoid that.

Fixes: nodejs#399
gibfahn added a commit to gibfahn/citgm that referenced this issue Apr 25, 2017
Eslint searches parent directories looking for config files until it
finds one with "root: true" in it. This sets "root: true" in CitGM's
tmpDir to avoid that.

Fixes: nodejs#399
gibfahn added a commit to gibfahn/citgm that referenced this issue Apr 25, 2017
Eslint searches parent directories looking for config files until it
finds one with "root: true" in it. This sets "root: true" in CitGM's
tmpDir to avoid that.

Fixes: nodejs#399
@gibfahn
Copy link
Member

gibfahn commented May 1, 2017

Suggested the root: true fix for the gulp config, and moved node into a subdirectory in the job. As an aside it probably always makes sense to use the clone into subdirectory option in the Jenkins Git plugin, that's the Git default so is probably more clear (and it avoids problems like this).

@gibfahn gibfahn closed this as completed Sep 17, 2017
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 a pull request may close this issue.

4 participants