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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: `report-needless-disables` argument is now exit with non-zero code. #2341

Merged
merged 1 commit into from Mar 26, 2017

Conversation

4 participants
@evilebottnawi
Member

evilebottnawi commented Feb 10, 2017

Which issue, if any, is this issue related to?

#2325

Is there anything in the PR that needs further explanation?

No, it's self explanatory.

It is very strange 馃槃 reportNeedlessDisables can be only true or false.

/cc @jeddy3 @davidtheclark

@davidtheclark

This comment has been minimized.

Contributor

davidtheclark commented Feb 10, 2017

It is very strange 馃槃 reportNeedlessDisables can be only true or false

@evilebottnawi Why is that strange?

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Feb 10, 2017

@davidtheclark reportNeedlessDisables can never be error

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Feb 10, 2017

@davidtheclark i bad know flow, what is wrong? (in CI)

lib/cli.js Outdated
@@ -207,10 +207,19 @@ Promise.resolve().then(() => {
return standalone(options)
}).then((linted) => {
if (reportNeedlessDisables) {
process.stdout.write(needlessDisablesStringFormatter(linted.needlessDisables))
if (reportNeedlessDisables === "error") {
const hasReportNeedlessDisable = linted.needlessDisables.some((sourceReport) => {

This comment has been minimized.

@davidtheclark

davidtheclark Feb 11, 2017

Contributor

Based on the type definitions, needlessDisables is not guaranteed to be on the linted object, so you can't call some on it without first checking whether it exists. An easy way to fix this with lodash, I think: _.get(linted, 'needlessDisables', []).some(..).

This comment has been minimized.

@davidtheclark

davidtheclark Feb 11, 2017

Contributor

Or, without lodash, const hasReportNeedlessDisable = !!linted.needlessDisables && linted.needlessDisables.some(..)

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Feb 16, 2017

@davidtheclark

The small change looks fine, @evilebottnawi. However, this is a breaking change, so needs to be merged into v8. Also, I think a note should be added to the documentation.

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Feb 16, 2017

@davidtheclark i think it is now breaking changes, because all error should return non zero exit code, i think it is fix

@davidtheclark

This comment has been minimized.

Contributor

davidtheclark commented Feb 16, 2017

Ehhh, maybe you're right. Didn't we have a semver policy doc? I can't find it!

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Feb 16, 2017

@davidtheclark ohh, i can't find too 馃槃
/cc @jeddy3 what do you think about semver doc and about this issue?

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Feb 16, 2017

However, this is a breaking change, so needs to be merged into v8

Yep, let's do that as there's a chance this change could break builds.

@davidtheclark

This comment has been minimized.

Contributor

davidtheclark commented Mar 15, 2017

@evilebottnawi Can you please change the target branch to v8, then we can merge?

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Mar 15, 2017

@davidtheclark yep, in this week

@evilebottnawi evilebottnawi force-pushed the issue-2325 branch 4 times, most recently from 40bf72d to 24981c6 Mar 21, 2017

@evilebottnawi evilebottnawi changed the base branch from master to v8 Mar 21, 2017

@evilebottnawi evilebottnawi changed the base branch from v8 to master Mar 21, 2017

@evilebottnawi evilebottnawi force-pushed the issue-2325 branch from 24981c6 to 1905873 Mar 21, 2017

@evilebottnawi evilebottnawi changed the base branch from master to v8 Mar 21, 2017

@evilebottnawi evilebottnawi force-pushed the issue-2325 branch from bd82001 to c39feab Mar 21, 2017

@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Mar 21, 2017

@jeddy3

jeddy3 approved these changes Mar 25, 2017

LGTM now that its heading into v8

@davidtheclark

This comment has been minimized.

Contributor

davidtheclark commented Mar 25, 2017

馃憤

@jeddy3 jeddy3 merged commit 366e426 into v8 Mar 26, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jeddy3 jeddy3 deleted the issue-2325 branch Mar 26, 2017

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Mar 26, 2017

v8 changelog:

  • Changed: report-needless-disables now exits with non-zero code (#2341).

jeddy3 added a commit that referenced this pull request Jul 14, 2017

8.0.0 (#2735)
* Add stylelint semantic versioning policy

* Fix whitespace and empty lines

* Add changelog entry

* Remove stylelint-config=standard reference

* Tweak major release optional options verbiage

* Update changelog

* Props ESLint

* Fix false negatives with ignore: ["descendant"] in selector-no-type (#2200)

* Update CHANGELOG.md

* Fix handling of shared-line comments (#2262)

* Fix handling of shared-line comments

Introduces a number of utilities and adjusts the following rules:
at-rule-empty-line-before, custom-property-empty-line-before,
declaration-empty-line-before, rule-nested-empty-line-before,
rule-non-nested-empty-line-before

Closes #2237; closes #2239; closes #2240.

* Fixes based on feedback

* Use getPreviousNonSharedLineCommentNode more

* Use more new helpers in comment-empty-line-before

* Add ignore: ["child"] option to selector-no-type (#2217)

* Add ignore: ["child"] option to selector-no-type

* Correct logic for ignore: ["child"] option

* Include line and column in reject tests with multiple type selectors

* Update CHANGELOG.md

* Report every selector resolved selector in selector-max-compound-selectors (#2350)

* Update CHANGELOG.md

* Add new shorthand data (#2354)

* Add new shorthand data

* Add mask shorthand

* Use defined list of properties in shorthand-property-no-redundant-values

* Improve declaration-block-no-shorthand-property-overrides speed a little bit

* Check prefixed properties agains prefixed properties in declaration-block-no-shorthand-property-overrides

* Improve declaration-block-no-redundant-longhand-properties speed a little bit

* Ignore prefixes case in declaration-block-no-shorthand-property-overrides

* Check prefixed properties agains prefixed properties in declaration-block-no-redundant-longhand-properties

* Add `grid-gap` to shorthand-property-no-redundant-values

* Explain properties set for shorthand-property-no-redundant-values

* Revert unnecessary changes

* Add 鈥渕ask鈥 to declaration-block-no-redundant-longhand-properties readme

* Update CHANGELOG.md

* Update changelog

* Remove deprecated rules (#2422)

* Remove deprecated rules

* Include `rule-empty-line-before` in system tests

* *-empty-line-before rules consider line as empty if it contains whitespace only (#2440)

* Update CHANGELOG.md

* Remove deprecated options (#2433)

* Update CHANGELOG.md

* report-needless-disables now exits with non-zero code (#2341)

* Update CHANGELOG.md

* Remove leftover files (#2442)

* Remove leftover file

* One more

* And one more

* Ignoring improvements (#2464)

* WIP ignoring improvements

* Remove standaloneIgnored property of result

* Add disableDefaultIgnores

* Add documentation tweaks

* Add parseStylelintIgnore tests

* Non-absolutized ignore file paths

* Restire gitignore syntax

* Remove parseStylelintIgnore

* Update CHANGELOG.md

* Check all linear-gradients in a value list (#2496)

Closes #2481

* Update CHANGELOG.md

* Update CHANGELOG.md

* Respect line case for `ignorePattern` (#2683)

Closes #2647

* Update CHANGELOG.md

* Update decls

* Fix remark warning

* Update snapshots

* Add missing message to reject test

* Fix tests for *-empty-line-before rules (#2688)

* Fix tests for *-empty-line-before rules

* Test addition and tweak

* Update to PostCSS@6 (#2561)

* Update to PostCSS@6

* Fix PostCSS@6 closing-brace test (#2689)

* Account for PostCSS@6's support for at-apply

* Remove only

* Fix postcssPlugin tests (#2690)

* Fix defaultSeverity.test.js test

* Fix ignoreDisables test

* Check ownSemiColon

* Fixed: remove hacks related to custom property set.

* Remove deprecated rules (#2693)

Closes #2521

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add VISION.md (#2704)

* Add releases.md (#2705)

* Add releases.md

Closes #2700

* Add dry-release step

* Use HTTPS for links

* Change ignore options for selector-max-type (#2701)

* Fix false negatives with ignore descendant

* Add ignore child

Closes #2699

* Update CHANGELOG.md

* Update README.md to account for VISION.md (#2727)

* Update README.md to account for VISION.md

* Fix lint warnings in CHANGELOG

* Update postcss-less to 1.0 (#2702)

* Update postcss-less to 1.0.2

* Update isStandardSyntaxRule for postcss-less 1.0

* Pass custom postcss stringifier when creating result

* Correct no-extra-semicolons Less test position

* Ignore Less mixins in no-extra-semicolons

* Update postcss-less to 1.1.0

* Correct no-extra-semicolons Less test position again

* Utilize isStandardSyntaxRule when checking for Less rules in no-extra-semicolons

* Case sensitive white/blacklists and ignore* options (#2709)

Closes #2660

* Update CHANGELOG.md

* docs: Add "New Release Issue Template" to releases.md (#2728)

* Exit with non-zero code on postcss warnings (#2713)

* Enhancement: exit with non zero code on postcss warnings.

* Account for parseErrors in stringformatqer

* Guard against no parseErrors

* Update CHANGELOG.md

* Refactor: simplify ignore logic. (#2732)

* Update changelog

* Bump 8.0.0 changes to top

* Correctly place changelog items

* Add introduction

* Add codefence for website

* Use relative path

* Include VISION in package for website

* Add link to VISION doc for website

* Changelog amends

* Update releases.md (#2738)

* Update eslint and config (#2737)

* Order requires

* Use eqeqeq eslint rule

* Update eslint

* Disable no-useless-escape just for this repo

* Enable no-useless-escape rule

* Use tilde

* Fix enable comment

* Add reject test for unescaped regex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment