tools: disallow mixed spaces and tabs for indents#5135
Closed
Trott wants to merge 2 commits intonodejs:masterfrom
Closed
tools: disallow mixed spaces and tabs for indents#5135Trott wants to merge 2 commits intonodejs:masterfrom
Trott wants to merge 2 commits intonodejs:masterfrom
Conversation
Member
|
A bit hard to tell that it's only about that one rule given the change, maybe this would be best as 2 commits? What if we jumped to disallowing tabs completely? I think that would be in line with style expectations anyway. |
Contributor
|
🔥 Down with the tabs! 🔥 |
Contributor
|
Maybe we should do away with the descriptions above each rule so it's more apparent that these lists are alphabetically sorted. They don't add much value and one can get a better description for each rule by using the links in the file. 😉 |
Trott
force-pushed
the
eslint-recommended
branch
from
ee6005f to
ce279d7
Compare
Member
Author
|
@rvagg I like your suggestion of splitting this into two commits. Done! |
Member
Author
With this new rule added, we would have tabs nearly completely disallowed, but not quite. See eslint/eslint#3521 for the explanation as to why ESLint doesn't support complete tab elimination without an external plugin. /shrug |
Member
Author
Yes, I like that idea. Although we should probably do it for all the lists in the |
Member
|
The test changes are in the wrong commit |
Rearrange the style rules in .eslintrc to be in alphabetical order. This has two benefits: It means the rules appear in the same order as they do in the ESLint documentation, easing cross-referencing. It also means that it is much easier to determine with visual inspection if a rule is set or not.
Enable eslint rule disallowing mixing tabs and spaces for indentation. Modify the one file that had been mixing tabs and spaces.
Trott
force-pushed
the
eslint-recommended
branch
from
ce279d7 to
d7dca68
Compare
Member
Author
|
Thanks, @targos! Fixed and force-pushed. |
Member
|
LGTM if CI is happy: https://ci.nodejs.org/job/node-test-pull-request/1588/ |
Member
Author
|
CI failure looks like it's actually due to #5109. We can re-run once that gets tidied up. |
Member
Author
|
CI is back in decent shape, so let's try this again: https://ci.nodejs.org/job/node-test-pull-request/1593/ |
Contributor
|
LGTM. Want to follow up with another PR to remove the comments? |
Trott
added a commit
that referenced
this pull request
Feb 9, 2016
Rearrange the style rules in .eslintrc to be in alphabetical order. This has two benefits: It means the rules appear in the same order as they do in the ESLint documentation, easing cross-referencing. It also means that it is much easier to determine with visual inspection if a rule is set or not. #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
Trott
added a commit
that referenced
this pull request
Feb 9, 2016
Enable eslint rule disallowing mixing tabs and spaces for indentation. Modify the one file that had been mixing tabs and spaces. PR-URL: #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
Member
Author
rvagg
pushed a commit
that referenced
this pull request
Feb 10, 2016
Rearrange the style rules in .eslintrc to be in alphabetical order. This has two benefits: It means the rules appear in the same order as they do in the ESLint documentation, easing cross-referencing. It also means that it is much easier to determine with visual inspection if a rule is set or not. #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
rvagg
pushed a commit
that referenced
this pull request
Feb 10, 2016
Enable eslint rule disallowing mixing tabs and spaces for indentation. Modify the one file that had been mixing tabs and spaces. PR-URL: #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Feb 18, 2016
Rearrange the style rules in .eslintrc to be in alphabetical order. This has two benefits: It means the rules appear in the same order as they do in the ESLint documentation, easing cross-referencing. It also means that it is much easier to determine with visual inspection if a rule is set or not. #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Feb 18, 2016
Enable eslint rule disallowing mixing tabs and spaces for indentation. Modify the one file that had been mixing tabs and spaces. PR-URL: #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Feb 18, 2016
Rearrange the style rules in .eslintrc to be in alphabetical order. This has two benefits: It means the rules appear in the same order as they do in the ESLint documentation, easing cross-referencing. It also means that it is much easier to determine with visual inspection if a rule is set or not. #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Feb 18, 2016
Enable eslint rule disallowing mixing tabs and spaces for indentation. Modify the one file that had been mixing tabs and spaces. PR-URL: #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
Merged
MylesBorins
pushed a commit
that referenced
this pull request
Mar 2, 2016
Rearrange the style rules in .eslintrc to be in alphabetical order. This has two benefits: It means the rules appear in the same order as they do in the ESLint documentation, easing cross-referencing. It also means that it is much easier to determine with visual inspection if a rule is set or not. #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Mar 2, 2016
Enable eslint rule disallowing mixing tabs and spaces for indentation. Modify the one file that had been mixing tabs and spaces. PR-URL: #5135 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
This was referenced Apr 28, 2023
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enable ESLlint rule disallowing mixing tabs and spaces for indentation.
Modify the one file that had been mixing tabs and spaces.
While at it, I rearranged the style rules in .eslintrc to be in
alphabetical order. This has two benefits: It means the rules appear
in the same order as they do in the ESLint documentation, easing
cross-referencing. It also means that it is much easier to determine
with visual inspection if a rule is set or not.