Skip to content

Allow ignoring warnings from configuration. #144

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

Conversation

blimmer
Copy link

@blimmer blimmer commented Oct 16, 2016

Since --quiet is not a supported configuration for the CLIEngine, I think this is how we'll have to address #142 .

Outstanding Questions:

  • manual testing - I could find examples of how to test this locally against a project. are there docs for this or can someone point me in the right direction?

* [ ] automated testing - is there any that can be added for this?
Integration testing seems out of scope for this change
* [ ] docs - where should this config option be documented?
docs website needs to be updated

@@ -209,6 +214,8 @@ function analyzeFiles() {
var path = result.filePath.replace(/^\/code\//, "");

result.messages.forEach(function(message) {
if (ignoreWarnings && message.severity === 1) { return; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming that 1 maps to the warning level but can we explicitly map this value to a named variable? Maybe up top as WARNING_LEVEL?

@dblandin
Copy link
Contributor

manual testing - I could find examples of how to test this locally against a project. are there docs for this or can someone point me in the right direction?

You can test your engine update locally with the CLI. You can either override the currently downloaded image for the eslint-3 channel or specify your own local engine name:

$ IMAGE_NAME=codeclimate/codeclimate-eslint:eslint-3 make image
eslint:
  enabled: true
  channel: eslint-3
$ codeclimate analyze
$ IMAGE_NAME=codeclimate/codeclimate-eslint-quiet make image
eslint-quiet:
  enabled: true
$ codeclimate analyze --dev

automated testing - is there any that can be added for this?

There are a few unit level specs in this repo but nothing that exercises the whole engine. I'd be happy with manual testing, but if you're willing to implement an integration spec, either in this PR or a following PR, I'd gladly review it :)

docs - where should this config option be documented?

This engine should really have a "Usage" section in the README which includes all of the user config options. At the moment, we have more thorough documentation on our docs site: https://docs.codeclimate.com/docs/eslint

The file, for now, shows how to create a local engine image for testing.
@blimmer blimmer force-pushed the feature/ignore-warnings branch from 00fb732 to 05e63bb Compare October 23, 2016 15:26
@blimmer
Copy link
Author

blimmer commented Oct 23, 2016

@dblandin this is ready for review again. RE the previous feedback, I've constant-ized the warning level as request and run some manual testing proving that this is working correctly.

With quiet turned off on one of my projects:

Analysis complete! Found 523 issues.

And with this config:

  eslint-quiet:
    enabled: true
    config:
     ignore_warnings: true

I get this (which is only reporting error levels eslint findings):

Analysis complete! Found 8 issues.

@blimmer
Copy link
Author

blimmer commented Oct 23, 2016

Also, I added a CONTRIBUTING.md file with the instructions you gave me to test locally against the CLI. I originally tried to find something like this before asking in the PR.

@blimmer
Copy link
Author

blimmer commented Oct 26, 2016

bump @dblandin - anything blocking this PR?

@dblandin
Copy link
Contributor

This looks great to me! We can address the README usage instructions in a separate PR.

Thanks!

@dblandin dblandin merged commit 625f55a into codeclimate:channel/eslint-3 Oct 26, 2016
@blimmer
Copy link
Author

blimmer commented Oct 27, 2016

Awesome, thanks. Very excited to see this feature go out. Is the README change something I can help with? Seems like it needs to be documented in a private repo for the docs site?

@dblandin
Copy link
Contributor

To update the usage instructions on our docs site, you can use the "Suggest Edits" feature at https://docs.codeclimate.com/docs/eslint. I'd also welcome a PR to this repository's README documenting the configuration option. The docs site usage instructions tend to be a bit broader than what we cover in engine READMEs.

blimmer pushed a commit to blimmer/codeclimate-eslint that referenced this pull request Oct 29, 2016
@blimmer blimmer deleted the feature/ignore-warnings branch October 29, 2016 20:30
@dblandin dblandin mentioned this pull request Nov 3, 2016
dblandin pushed a commit that referenced this pull request Nov 7, 2016
pointlessone pushed a commit that referenced this pull request Feb 23, 2017
pointlessone pushed a commit that referenced this pull request Feb 23, 2017
pointlessone pushed a commit that referenced this pull request Feb 23, 2017
gdiggs pushed a commit that referenced this pull request Feb 28, 2017
* Monkey patch eslint, instead of modifying it

- 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

* Use eslint-2.4.0

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.

* Comment loadPackage override

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)

* Update babel-eslint to v6

Fixes #91

* Update dependencies

```
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
```

* Add standard style plugin and shared config

* Vendor additional Standard Style config packages (#102)

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

* Add eslint-config-google as available plugin

* Added supporto for ESLint plugin Flowtype

* Add support for eslint-config-angular

* Add task to update npm-shrinkwrap.json

* Update package.json (#112)

eslint-2: Vendor eslint-plugin-angular package

* add eslint-config-ember package

* Switch base image to avoid Segfault

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.

* Update babel-eslint & eslint-flowtype

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.

* Add --gecos "" to avoid adduser prompt

* add hapi config support

hapi eslint config is based on eslint-config-hapi
and eslint-plugin-hapi packages

* Add eslint-config-airbnb-base as a top-level dep

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.

* Update circle to trigger build

* Upgrade to Node 6.5.0

* Update packages for ESLint 3

* eslint-3: Added ESLint plugins (#131)

* eslint-3: Added ESLint plugins

* Added appropriate eslint-plugin rule categorizations

* Shifted category mappings to individual JSON file

* Alphabetized ESLint plugin rule category mapping

* Bump eslint to 3.6.1 & eslint-plugin-import to 1.16.0 (#136)

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.

* Support eslint-plugin-ember-suave. (#140)

* Allow ignoring warnings from configuration.

* Add CONTRIBUTING.md.
The file, for now, shows how to create a local engine image for testing.

* Add ignore_warnings to README

As requested in #144.

* Update README.md

* Update project README

- Update project/documentation links
- Use open source Code Climate badge
- Hard-wrap text
- Fix YAML identation

* Upgrade ESLint & some packages

* 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.

* Add support for eslint-plugin-react-intl plugin (#154)

* chore(packages): upgrade eslint

* docs(CONTRIBUTING): fix .codeclimate.yml example

* feat(config): add eslint-config-simplifield

* Switch to yarn for installing dependencies

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.

* Upgrade eslint-config-google to 0.7.1

* Add eslint-plugin-meteor

Close #155

Just ran: `bin/yarn add eslint-plugin-meteor`

* Add eslint-plugin-immutable

`bin/yarn add eslint-plugin-immutable`

* Add eslint-plugin-import-order

`bin/yarn add eslint-plugin-import-order`

* Add eslint-plugin-jasmine

`bin/yarn add eslint-plugin-jasmine`

* Add eslint-config-semistandard

`bin/yarn add eslint-config-semistandard`

* Yarn add eslint-config-react-app
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

* added eslint-plugin-xogroup plugin

* Upgrade eslint

```
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

* add support for eslint prettier plugin

* Add config upgrader

Automatically upgrade pre-ESLint 3 config to ESLint 3-compatible format.

* Fix null-config issue with upgrader

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.
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.

2 participants