Update lint rules to match @peggyjs/eslint-config@3.1.0. #349
Merged
hildjj merged 3 commits intopeggyjs:mainfrom Mar 21, 2023
Merged
Update lint rules to match @peggyjs/eslint-config@3.1.0. #349hildjj merged 3 commits intopeggyjs:mainfrom
hildjj merged 3 commits intopeggyjs:mainfrom
Conversation
hildjj
force-pushed
the
update-lint
branch
2 times, most recently
from
db2d723 to
99ed7e6
Compare
Mingun
previously approved these changes
Feb 26, 2023
Member
Mingun
left a comment
Mingun
left a comment
There was a problem hiding this comment.
Generally approve, although no-shadow, in my mind, are not so useful.
hildjj
force-pushed
the
update-lint
branch
3 times, most recently
from
81396c4 to
274845b
Compare
Contributor
Author
|
I really like |
reverofevil
reviewed
Mar 20, 2023
reverofevil
reviewed
Mar 20, 2023
| return "/^[" | ||
| + (cls.inverted ? "^" : "") | ||
| + cls.value.map(part => Array.isArray(part) | ||
| + cls.value.map(part => (Array.isArray(part) |
There was a problem hiding this comment.
I'd rather disable the eslint rule instead. IMO this code was completely fine.
Contributor
Author
There was a problem hiding this comment.
I actually like that it's clearer now.
Contributor
Author
There was a problem hiding this comment.
Ironically, I put the parens in the WRONG PLACE the first time. :)
reverofevil
reviewed
Mar 20, 2023
| expect(parser).to.parse("42*43+44*45+46*47+48*49", 42 * 43 + 44 * 45 + 46 * 47 + 48 * 49); | ||
| expect(parser).to.parse("42*43-44*45", 42 * 43 - 44 * 45); | ||
| expect(parser).to.parse("42*43-44*45-46*47-48*49", 42 * 43 - 44 * 45 - 46 * 47 - 48 * 49); | ||
| expect(parser).to.parse("42*43+44*45", (42 * 43) + (44 * 45)); |
There was a problem hiding this comment.
So even basic arithmetic priorities should be highlighted with parens? :)
Contributor
Author
There was a problem hiding this comment.
Yep. I actually agree with this.
Contributor
Author
|
note that I messed something up and the tests don't pass. Fixing that now. |
Mingun
previously approved these changes
Mar 21, 2023
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
3 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.
Still need to update package.json and package-lock.json once that version is shipped.