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

Add eslint-ratchet #239

Merged
merged 19 commits into from
Mar 15, 2018
Merged

Add eslint-ratchet #239

merged 19 commits into from
Mar 15, 2018

Conversation

marchuffnagle
Copy link
Contributor

There is a Ruby gem called quality that runs a variety of variety of static code analysis tools against your Ruby code. One of the interesting features of that gem is that it records the number of errors in your code. If you make a change which increases the number of errors, it complains. If you make a change which decreases the number of errors, it records that lower number, and that is the new upper limit. By doing that, it causes the number of errors to trend down over time.

We talked about doing something similar in the Cumulus architecture meeting yesterday to address the large number of eslint errors that are generated in our code. This PR replicates some of the functionality that the quality gem provides.

This PR introduces a bash script called bin/eslint-ratchet. That script runs eslint against your code and determines the number of errors that eslint has found. It also reads the previous best score from .eslint-ratchet-high-water-mark. If there are more errors now than before, it fails and indicates that there has been an increase in the number of errors. If there are less errors than before, it updates .eslint-ratchet-high-water-mark with that new and improved number. That updated .eslint-ratchet-high-water-mark file should be checked into git.

Circle CI also runs eslint-ratchet. It expects the number of errors reported to match the number in the .eslint-ratchet-high-water-mark file. If it does not, it will fail the CI run. That means that you either 1) introduced new errors or 2) forgot to run eslint-ratchet and commit .eslint-ratchet-high-water-mark with the new, lower score.

Copy link
Contributor

@scisco scisco left a comment

Choose a reason for hiding this comment

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

If this requires a new step in the PR process for every developer, we should update the README and and the PR template

@marchuffnagle marchuffnagle merged commit 32515c9 into master Mar 15, 2018
@marchuffnagle marchuffnagle deleted the eslint-ratchet branch March 15, 2018 15:57
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.

3 participants