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.
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
Add no-invalid-position-at-import-rule #5202
Add no-invalid-position-at-import-rule #5202
Changes from 7 commits
b498c0c
1e3a407
3c34558
44d7556
27e4fb8
89f70d1
77bf542
5f3d454
c2ff6b1
9eaee88
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[IMO] It seems a bit readable to me if using
toLowerCase
. But this is just my opinion, so you can ignore it. 😃There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion looks suitable. Just made some research and
toLowerCase()
used much more often in rules.@jeddy3 what do you think about it? Do we have any convention on this or should create one if not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the same thought about
toLowerCase
as @ybiquitous when I was first reviewing the pull request as we've used that approach in loads of places. However, the regex approach sort of grew on me because we use it for filtering at-rules and declarations when walking (like we did in the last rule we added), so I didn't mention it.I'm all for conventions, especially within rules, because they're such a time saver when reviewing code.
I'm not sure which of the approaches is more readable, though. So, let's go with @ybiquitous's
toLowerCase
suggestion for now to be consistent with how we're treating names and values in other rules. We can then, if we want, refactor across the board later down the line.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following on, we'll probably stick with
toLowerCase
(for names and values) because (having looked myself) we often: