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

[#2829] add autofix for fix-shorthand-property-no-redundant-values #2956

Merged
merged 2 commits into from
Oct 12, 2017
Merged

[#2829] add autofix for fix-shorthand-property-no-redundant-values #2956

merged 2 commits into from
Oct 12, 2017

Conversation

modosc
Copy link
Member

@modosc modosc commented Oct 12, 2017

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

#2829

Is there anything in the PR that needs further explanation?

No, it's self explanatory

Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

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

Looks good, except few small things:

  1. Please, add fix: true for all testRule to be sure, we don't break style sheets unintentionally.
  2. Add "(Autofixable)" for this rule in the rules list.

result,
ruleName
});
if (context.fix) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add an empty line before if for better readability, please?

@@ -18,6 +18,8 @@ This rule alerts you when you use redundant values in the following shorthand pr
- `border-width`
- `grid-gap`

The `--fix` option on the [command line](../../../docs/user-guide/cli.md#autofixing-errors) can automatically fix some of the problems reported by this rule.
Copy link
Member

Choose a reason for hiding this comment

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

can automatically fix some of the problems reported by this rule

Looks like it could fix all problems.

@modosc
Copy link
Member Author

modosc commented Oct 12, 2017

ok - addressed all the feedback.

Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

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

Please, remove unrelated changes. Everything else looks good.


#### String

- [`string-quotes`](../../lib/rules/string-quotes/README.md): Specify single or double quotes around strings.

#### Length

- [`length-zero-no-unit`](../../lib/rules/length-zero-no-unit/README.md): Disallow units for zero lengths.
- [`length-zero-no-unit`](../../lib/rules/length-zero-no-unit/README.md): Disallow units for zero lengths (Autofixable).
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR. Addressed in #2957.

- [`number-leading-zero`](../../lib/rules/number-leading-zero/README.md): Require or disallow a leading zero for fractional numbers less than 1.
- [`number-no-trailing-zeros`](../../lib/rules/number-no-trailing-zeros/README.md): Disallow trailing zeros in numbers.
- [`number-leading-zero`](../../lib/rules/number-leading-zero/README.md): Require or disallow a leading zero for fractional numbers less than 1 (Autofixable).
- [`number-no-trailing-zeros`](../../lib/rules/number-no-trailing-zeros/README.md): Disallow trailing zeros in numbers (Autofixable).
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR. Addressed in #2957.

@modosc
Copy link
Member Author

modosc commented Oct 12, 2017

should be good now?

Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

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

Thank you!

@modosc modosc merged commit 44e0745 into stylelint:master Oct 12, 2017
@hudochenkov
Copy link
Member

Reviewing pull requests:

Merge simple documentation fixes after one approval and all other pull requests after two approvals.

@modosc Don't violate this rule!

@modosc
Copy link
Member Author

modosc commented Oct 12, 2017

@modosc Don't violate this rule!

sorry about that.

@jeddy3
Copy link
Member

jeddy3 commented Oct 13, 2017

Retro-active review: Code LGTM!

@modosc Thanks for adding the changelog item. Please remember to respect the conventions in the guide:

  • ordering by group name and by rule name.
  • consistent spacing and pronunciation.

And then posting the changelog item into the thread.

I've tweak it to:

  • Added: shorthand-property-no-redundant-values autofix (#2956).

@modosc
Copy link
Member Author

modosc commented Oct 13, 2017

@hudochenkov @jeddy3 thanks for bearing with me - i opened #2960 to clarify the approval process a bit.

@jeddy3
Copy link
Member

jeddy3 commented Oct 13, 2017

thanks for bearing with me

No worries at all. Many thanks for your continued work on stylelint. You've been nailing the autofix stuff recently :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants