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

Fix false negatives for nested at-rules in selector-max-* #3959

Merged
merged 10 commits into from Apr 3, 2019

Conversation

4 participants
@ccabrales
Copy link
Contributor

commented Feb 20, 2019

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

Closes #3947

Is there anything in the PR that needs further explanation?

This seems to solve the problem for selector-max-type, but I wanted to put up a POC before moving on to the other rules to see if I'm moving in the right direction. Feedback is appreciated.

.gitignore Outdated
@@ -4,3 +4,4 @@ node_modules
.eslintcache
package-lock.json
yarn.lock
**/.idea/

This comment has been minimized.

Copy link
@evilebottnawi

evilebottnawi Feb 20, 2019

Member

Just use .idea

This comment has been minimized.

Copy link
@hudochenkov

hudochenkov Feb 20, 2019

Member

It shouldn't be in this project .gitignore, because it's not related to stylelint directly. How to setup global .gitignore: https://gist.github.com/subfuzion/db7f57fff2fb6998a16c

This comment has been minimized.

Copy link
@ccabrales

ccabrales Feb 20, 2019

Author Contributor

🤷‍♂️ sure I'll remove it. Makes no difference to me

Casey Cabrales
@jeddy3

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

@ccabrales Thanks for so much for making a start on this! The POC looks good to me and the tests give me confidence. I think we can roll the change out to the other selector-max-* rules.

@jeddy3 jeddy3 referenced this pull request Feb 21, 2019

Closed

Release 10.0.0 #3954

6 of 6 tasks complete

@ccabrales ccabrales changed the title POC for fixing false negatives for nested at-rules in selector-max-* Fix false negatives for nested at-rules in selector-max-* Feb 22, 2019

@ccabrales

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

@jeddy3 I updated the other selector-max-* rules where needed, and added some tests for ones that didn't need code updates for future-proofing them. Let me know if I need to make any tweaks.

@jeddy3

jeddy3 approved these changes Feb 24, 2019

Copy link
Member

left a comment

@ccabrales Excellent work! Thanks for updating the other selector-max-* rules.

Looks good to me!

@ccabrales

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2019

@jeddy3 My pleasure! Is there a timeline on getting final approval and into a release?

@jeddy3

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Is there a timeline on getting final approval and into a release?

It's just waiting on another team member to find the time to review it. It looks like everyone is pretty busy of late. If no one finds time by the weekend, I'll merge it then so it's in the next release.

@ccabrales

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

@jeddy3 Hey, wanted to check in for an update?

@hudochenkov
Copy link
Member

left a comment

Good job! Thank you!

@hudochenkov hudochenkov merged commit 843eea6 into stylelint:master Apr 3, 2019

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.009%) to 96.373%
Details
@hudochenkov

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

  • Fixed: selector-max-* false negatives for nested at-rules (#3959).
@jeddy3

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

Hey, wanted to check in for an update?

@ccabrales Sorry about the delay getting this in. We're all been busy of late. We're planning to do a release in a week and a half's time. You can track the progress of that in #3954

@ccabrales ccabrales deleted the ccabrales:issue-3947 branch Apr 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.