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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not fail yarn linc for ignored file warning (#11615) #11641

Merged
merged 6 commits into from
Nov 28, 2017
Merged

Do not fail yarn linc for ignored file warning (#11615) #11641

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6d27589
Add rule to ignore default handling of not linting hidden files
HeroProtagonist Nov 23, 2017
efc625f
Undo changes
HeroProtagonist Nov 26, 2017
c56595f
Add function to validate warnings
HeroProtagonist Nov 26, 2017
8c9784f
Use validateWarnings when reporting linc command
HeroProtagonist Nov 26, 2017
cf2a4a9
Restore files
HeroProtagonist Nov 28, 2017
c6d0e69
Contain code to line file
HeroProtagonist Nov 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion scripts/eslint/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,22 @@ const CLIEngine = require('eslint').CLIEngine;
const cli = new CLIEngine();
const formatter = cli.getFormatter();

module.exports = function lintOnFiles(filePatterns) {
function lintOnFiles(filePatterns) {
const report = cli.executeOnFiles(filePatterns);
const formattedResults = formatter(report.results);
console.log(formattedResults);
return report;
}

function validWarnings(report) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this to where it's used?
Also let's rename to something like getSourceCodeWarnings.

In fact I would just inline this logic in the script that uses it, and add a comment explaining why it exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes reflect this. Do you prefer to have function declaration vs definition?

const {warningCount, results} = report;
const ignoreWanings = results.filter(
({messages}) => messages[0] && messages[0].message.includes('File ignored')
);
return warningCount > 0 ? warningCount !== ignoreWanings.length : false;
}

module.exports = {
lintOnFiles,
validWarnings,
};
2 changes: 1 addition & 1 deletion scripts/tasks/eslint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

'use strict';

const lintOnFiles = require('../eslint');
const {lintOnFiles} = require('../eslint');
const report = lintOnFiles(['.']);
if (report.errorCount > 0 || report.warningCount > 0) {
console.log('Lint failed.');
Expand Down
4 changes: 2 additions & 2 deletions scripts/tasks/linc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@

'use strict';

const lintOnFiles = require('../eslint');
const {lintOnFiles, validWarnings} = require('../eslint');
const listChangedFiles = require('../shared/listChangedFiles');

const changedFiles = [...listChangedFiles()];
const jsFiles = changedFiles.filter(file => file.match(/.js$/g));

const report = lintOnFiles(jsFiles);
if (report.errorCount > 0 || report.warningCount > 0) {
if (report.errorCount > 0 || validWarnings(report)) {
console.log('Lint failed for changed files.');
process.exit(1);
} else {
Expand Down