Skip to content

ESLint 3 preparation for master #178

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

Merged
merged 47 commits into from
Feb 28, 2017
Merged

ESLint 3 preparation for master #178

merged 47 commits into from
Feb 28, 2017

Conversation

pointlessone
Copy link
Contributor

@pointlessone pointlessone commented Feb 23, 2017

This is a preparation of channel/eslint-3 branch for merge into master. The HEAD of this branch is effectively the same as channel/eslint-3.

This branch contains all commits from ESLint 3 branch except for merge commits and shared commits. Some commits are a bit different to the original ones because of conflict resolution during cherry-picking.

Because I cherry-picked commits git/GH shows me as the committer on all these commits. It's possible to amend commits so that their committers were the same as authors. Let me know if it's important.

Dan Rumney and others added 30 commits February 23, 2017 13:26
- Add access to ESLint docs
- Add support for parametrized Docker builds, building different
  versions of the documentation
- Automatically detect which version of the docs we should be including
Fixes:

    npm WARN eslint-config-airbnb@6.2.0 requires a peer of eslint@^2.4.0 but none was installed.
    npm WARN codeclimate-eslint@0.0.3 No license field.
    Cloning into 'eslint'...
    npm ERR! peer dep missing: eslint@^2.4.0, required by eslint-config-airbnb@6.2.0
    npm ERR! code 1
    error: pathspec 'vnull' did not match any file(s) known to git.
Fixes:

    /usr/src/app/node_modules/meld/meld.js:67
                      if (typeof target[pointcut] === 'function') {
                                        ^

    TypeError: Cannot read property 'loadPackage' of undefined
        at meld (/usr/src/app/node_modules/meld/meld.js:67:23)
        at Function.around (/usr/src/app/node_modules/meld/meld.js:436:12)
        at patcher (/usr/src/app/lib/eslint-patch.js:21:8)
        at Object.<anonymous> (/usr/src/app/bin/eslint.js:11:44)
        at Module._compile (module.js:397:26)
        at Object.Module._extensions..js (module.js:404:10)
        at Module.load (module.js:343:32)
        at Function.Module._load (module.js:300:12)
        at Function.Module.runMain (module.js:429:10)
        at startup (node.js:139:18)
```
eslint@2.4.0               -> eslint@2.10.2
eslint-config-airbnb@6.0.2 -> eslint-config-airbnb@9.0.1
eslint-plugin-babel@2.1.1  -> eslint-config-airbnb@3.2.0
eslint-plugin-react@4.0.0  -> eslint-config-react@5.1.1
```
Many users using [Standard Style][] also use these configuration packages when
writing React/JSX.

[Standard Style]: https://github.com/feross/standard

This commit vendors the [eslint-config-standard-react] and
[eslint-config-standard-jsx] packages.

[eslint-config-standard-react]: https://github.com/feross/eslint-config-standard-react
[eslint-config-standard-jsx]: https://github.com/feross/eslint-config-standard-jsx
eslint-2: Vendor eslint-plugin-angular package
Related to #111, a Segfault occurs when we hammer STDOUT with issues.
Switching from alpine to a debian-based image seems to behave better in
this scenario.
We're a bit behind on these. Flowtype in particular has new rules we
weren't supporting.

Looks like we hadn't actually filled in the shrinkwrap file before now,
either, so that's done.
hapi eslint config is based on eslint-config-hapi
and eslint-plugin-hapi packages
eslint-config-airbnb-base is a transitive dependency of
eslint-config-airbnb, but it is useful in its own right and should be
included at the top level such that it has proper support in the eslint
engine.
* eslint-3: Added ESLint plugins

* Added appropriate eslint-plugin rule categorizations

* Shifted category mappings to individual JSON file

* Alphabetized ESLint plugin rule category mapping
The eslint bump was necessitated by `airbnb-config-base` wanting it.
There's also a 2.0.0 of `eslint-plugin-import` available, but attempting
to jump that far gave some peer package errors, so maybe we can't
support that yet.
The file, for now, shows how to create a local engine image for testing.
- Update project/documentation links
- Use open source Code Climate badge
- Hard-wrap text
- Fix YAML identation
* We're several point releases behind on ESLint
* After updating just that, I got some errors from shrinkwrap for two
  other packages we support, so I upgraded those too.
leftees and others added 16 commits February 23, 2017 17:44
This seems more intuitive to me today, and I hope will make it easier
for contributors who want to add or upgrade plugins.

Note: this is using nightly yarn, because there's a fix for Circle CI +
Docker + Yarn compatibility that is made but not officially released
yet. We can switch to stable yarn once 0.18.0 leaves pre-release. The
fix was PR 1837 on the Yarn repo.
Close #155

Just ran: `bin/yarn add eslint-plugin-meteor`
`bin/yarn add eslint-plugin-immutable`
`bin/yarn add eslint-plugin-import-order`
`bin/yarn add eslint-plugin-jasmine`
`bin/yarn add eslint-config-semistandard`
Yarn upgrade babel-eslint ^7.0.0
Yarn upgrade eslint-plugin-flowtype to ^2.21.0
Yarn upgrade eslint-plugin-import to ^2.0.1
Yarn upgrade eslint-plugin-jsx-a11y to ^2.2.3
Yarn upgrade eslint-plugin-react to ^6.4.1
```
make
bin/yarn upgrade eslint
```

We'd like to pull in a fix for a bug that prevents using this rule:
http://eslint.org/docs/2.0.0/rules/generator-star-spacing
Automatically upgrade pre-ESLint 3 config to ESLint 3-compatible format.
@pointlessone
Copy link
Contributor Author

I'm not sure what to do with CC failure as this PR effectively has no new code. It just moves it around a bit.

@pointlessone pointlessone requested a review from gdiggs February 23, 2017 15:51
@maxjacobson
Copy link
Contributor

That sounds like a good opportunity to use the PR approval feature :)

@gdiggs
Copy link
Contributor

gdiggs commented Feb 23, 2017

This LGTM - we should wait to merge it until @maxjacobson resolves the upgrader issue

When upgrader encountered a project that had no .eslintrc* but had an
unrelated package.json that had no ESLint-related options in it an
exception was thrown.
Copy link
Contributor

@gdiggs gdiggs left a comment

Choose a reason for hiding this comment

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

Testing this image seems to work! I will merge/release this tomorrow

@gdiggs gdiggs merged commit 296d78e into master Feb 28, 2017
@gdiggs gdiggs deleted the master-eslint-3 branch February 28, 2017 15:58

module.exports = function patcher(eslint) {

meld.around(eslint.CLIEngine, 'loadPlugins', function(joinPoint) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@gordondiggs @pointlessone Hey guys! I am trying to understand the reasons for this. Digging at the eslint source, it seems the loadPlugings function was removed on version 2.0, we are currently supporting version 3.14.1... is this a leftover from some experimentation?

Copy link
Contributor

Choose a reason for hiding this comment

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

This was likely added when we supported ESLint 2 on the way to supporting ESLint 3. Is there a similar function in ESLint 3 that we can meld?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the history it seems to be added while upgrading to 3, that's why I am confuse.
Anyways I am digging into eslint to understand where did this logic go.

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.