Skip to content

update eslint fork to release 1.10.3 of eslint #50

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

Closed
wants to merge 2 commits into from

Conversation

ABaldwinHunter
Copy link
Contributor

Full changes here:
codeclimate/eslint@d24d0b4

Original impetus was an error caused by eslint out of date on a snapshot.

Note that in addition to regular updates, this change to our eslint fork required
rewriting the line that permits airbnb packages but no others.

cc @codeclimate/review

Full changes here:
codeclimate/eslint@d24d0b4

Original impetus was an error caused by eslint out of date on a snapshot.

Note that in addition to regular updates, this change to our eslint fork required
rewriting the line that permits airbnb packages but no others.
@ABaldwinHunter
Copy link
Contributor Author

p.s. ran CLI with updated eslint engine on repo that had had broken snapshot and analysis worked.

...
== modules/contrib/mollom/js/mollom.js (3 issues) ==
1: Use the function form of "use strict". [eslint]
3: Expected indentation of 2 space characters but found 0. [eslint]
10: Expected indentation of 2 space characters but found 0. [eslint]

== themes/atletico/js/navigation.js (6 issues) ==
1: Use the function form of "use strict". [eslint]
3: Strings must use singlequote. [eslint]
5: Missing space before function parentheses. [eslint]
7: Missing space before function parentheses. [eslint]
7: Missing space before opening brace. [eslint]
8: Keyword "if" must be followed by whitespace. [eslint]

Analysis complete! Found 2,523 issues.

@@ -9,7 +9,7 @@
},
"dependencies": {
"babel-eslint": "4.1.3",
"eslint": "codeclimate/eslint.git#9dba1fe",
"eslint": "codeclimate/eslint.git#d24d0b451b75cfad304faeb394371096f2c90777",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the shortened sha here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for preferring the shortened sha? I went the other way in a recent csslint bump with the following reasoning: This makes for easier editing (git log or the GitHub UI most often gives you full shas).

Copy link
Contributor

Choose a reason for hiding this comment

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

No real reason - I brought it up because that's what we've done historically here

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. I don't really care, just curious.

@ABaldwinHunter
Copy link
Contributor Author

@gordondiggs fixed.

I'm going to close and re-open this PR from a fork for cross-repo testing.

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