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 number-no-trailing-zeros #2947

Merged
merged 1 commit into from Oct 10, 2017

Conversation

3 participants
@modosc
Member

modosc commented Oct 9, 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

@modosc modosc referenced this pull request Oct 9, 2017

Closed

Add autofix to rules for parity with stylefmt #2829

19 of 21 tasks complete
@jeddy3

jeddy3 approved these changes Oct 10, 2017

@modosc Thanks! This LGTM. It seems to address the two concerns raised with the number-leader-zero implementation:

  • explicitly checking the node.type.
  • mutating the AST outside of the value-parser loop.
}
};
};
function removeTrailingZeros(input, startIndex, endIndex) {

This comment has been minimized.

@evilebottnawi

evilebottnawi Oct 10, 2017

Member

Maybe best place utils?

This comment has been minimized.

@jeddy3

jeddy3 Oct 10, 2017

Member

Is it used by more than rule? If so, then yes I agree. Otherwise, it's probably more helpful to maintainers to keep it here.

@modosc modosc merged commit 445d0bf into stylelint:master Oct 10, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 95.681%
Details

@modosc modosc deleted the modosc:number-no-trailing-zeros branch Oct 10, 2017

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Oct 10, 2017

@modosc Thanks again for this.

Please remember to update the CHANGELOG when merging a PR as it's easy for us to loose track.

I did this one for you in 2277cb7

  • Added: number-no-trailing-zeros autofix (#2947).
@modosc

This comment has been minimized.

Member

modosc commented Oct 10, 2017

@jeddy3 does the changelog update become part of the pull request or happen after the merge?

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Oct 10, 2017

does the changelog update become part of the pull request or happen after the merge?

After the merge (as it helps us avoid conflicts). I just realised that the guidelines aren't that clear. I'll tweak them now.

@modosc

This comment has been minimized.

Member

modosc commented Oct 10, 2017

After the merge (as it helps us avoid conflicts). I just realised that the guidelines aren't that clear. I'll tweak them now.

got it - it would be cool to use something to do this automatically, maybe https://skywinder.github.io/github-changelog-generator/ ?

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Oct 10, 2017

got it - it would be cool to use something to do this automatically, maybe https://skywinder.github.io/github-changelog-generator/ ?

We experimented with automating it, but most things ended up being more trouble than they're worth. I think the current process has little overhead.

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