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

Fixed issue when "autofix" doesn't fix all files. #93

Closed
wants to merge 2 commits into from
Closed

Fixed issue when "autofix" doesn't fix all files. #93

wants to merge 2 commits into from

Conversation

blobor
Copy link
Contributor

@blobor blobor commented Sep 26, 2015

If you haven't setted "maxErrors" property in your config file, it JSCS uses defaults (50). So in case we run task with "fix" setted to "true" we could only fix 50 errors.

I fixed this problem by setting "Infinity" value in case of fix is setted to true.

@blobor
Copy link
Contributor Author

blobor commented Oct 1, 2015

Could somebody review this PR ?

overrides.maxErrors = Infinity;
}

checker.getConfiguration().override(overrides);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just config.maxErrors = Infinity; above and drop this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case of just validation (not fixing), consumer could configure maximum count of errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

But that's already handled by the above if statement. It will only override when opts.fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, you are right.

@sindresorhus
Copy link
Contributor

I think this is something that should be eventually fixed in core JSCS, though. @blobor Would you mind opening an issue over there?

@blobor
Copy link
Contributor Author

blobor commented Oct 2, 2015

@sindresorhus, JSCS team already have similar issue.
They fixed it at configuration stage.

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.

None yet

2 participants