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

Dont lint ignored files #86

Merged
merged 3 commits into from
Mar 19, 2017
Merged

Conversation

LucasHill
Copy link
Contributor

This is to remove some confusion around ignored files. Currently, all JS files will show up in the linter as "passed eslint" even if they were ignored. This changes that behavior and if a file is ignored, it never gets run through the linter, and doesn't show up in the test report.

@@ -21,7 +21,7 @@ const FIXTURES_PATH_ESLINTIGNORE_FOR_WARNING = path.resolve(process.cwd(), './te
const FIXTURES_PATH_ESLINTRC = path.join(FIXTURES_PATH, '.eslintrc.js');
const FIXTURES_PATH_ESLINTRC_ALTERNATE = path.join(FIXTURES_PATH, '.eslintrc-alternate.js');
const FIXTURE_FILE_PATH_ALERT = 'fixtures/alert.js';
const JS_FIXTURES = fs.readdirSync(FIXTURES_PATH).filter((name) => /\.js$/.test(name));
const JS_FIXTURES = fs.readdirSync(FIXTURES_PATH).filter((name) => /\.js$/.test(name) && !/^.eslintrc/.test(name));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before it was actually linting the eslintrc files. The eslint API considers these an "ignored file" by default, and they are no longer linted, so the count of JS_FIXTURES needed to be reduced to exclude them.

@@ -31,32 +30,6 @@ function getResultSeverity(resultMessages = []) {
}, 0);
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole filtering logic is superseded by the new guard which keeps ignored files from being linted on at all. So it is no longer necessary. I kept the tests in to ensure however.

@Turbo87
Copy link
Member

Turbo87 commented Mar 19, 2017

LGTM, thanks!

@Turbo87 Turbo87 merged commit c18b05c into ember-cli:master Mar 19, 2017
@Turbo87 Turbo87 added the bug label May 1, 2017
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.

None yet

2 participants