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

WEB-1417-updating styelint to 14.0.1 #7

Merged
merged 48 commits into from Nov 12, 2021

Conversation

nesamouzehkesh
Copy link
Contributor

@nesamouzehkesh nesamouzehkesh commented Oct 26, 2021

This PR aims to update the stylelint from v10.0.1 to v13.13.1

Rules that were newly added to stylelint but I was not really sure we need them:

  • hue-degree-notation (added in 13.5.0)
  • color-function-notation (added in 13.5.0)
  • alpha-value-notation (added in 13.5.0)
    UPDATE: the above have now been added.

Also there are some new config keys available now (as of 13.10.0) such as:

  • ignoreDisables
  • reportNeedlessDisables
  • reportDescriptionlessDisables
  • reportInvalidScopeDisables

that the stylelint team was so excited about (:D), so this is a reference to their excitement! Ummm, I don't really understand the need for or the importance of these 4 config keys so if anyone could explain it to me that would be awesome. Also the issue link is here.
UPDATE: the above have now been added.

With 14.0.0 there has been a secondary option that can be used for any rule, exactly like severity and this new one is called disableFix; It was introduced because sometimes we need to find error which should not be automatically fixed, or it was automatically fixed by mistake, instead, we want to fix it manually. 

Example:

a {
        color: red; // two indentations before color
   }

And your rule being:
indentation: [2, {disableFix: true}]

Then stylelint would error on this one because it has not been auto fixed.The example on indentation though does not make much sense! But are there any rules among the rules that we have that you guys would think we want this to be enforced?

Also with 14.0.0, they have removed the Node.js 10 support, I assume we use 14 so I assume this won’t be an issue but just highlighting this here in case I am missing something.

Other than the above I will elaborate on the changes that have been added in this PR right where they have been added.

Sample usage of this upgrade in Infinity

Sample usage of this upgrade in Ionic

NOTE: aiming to go ahead with merging and versioning this one after the upgrade in Flash Widget (In Progress...)

rules/stylistic-issues.js Outdated Show resolved Hide resolved
rules/limit-language-features.js Show resolved Hide resolved
rules/stylistic-issues.js Outdated Show resolved Hide resolved
rules/limit-language-features.js Outdated Show resolved Hide resolved
rules/possible-errors.js Outdated Show resolved Hide resolved
@mingteo-str
Copy link
Contributor

This PR aims to update the stylelint from v10.0.1 to v13.13.1

Rules that were newly added to stylelint but I was not really sure we need them:

* `hue-degree-notation` (added in 13.5.0)

* `color-function-notation` (added in 13.5.0)

* `alpha-value-notation` (added in 13.5.0)

Also there are some new config keys available now (as of 13.10.0) such as:

* `ignoreDisables`

* `reportNeedlessDisables`

* `reportDescriptionlessDisables`

* `reportInvalidScopeDisables`

that the stylelint team was so excited about (:D), so this is a reference to their excitement! Ummm, I don't really understand the need for or the importance of these 4 config keys so if anyone could explain it to me that would be awesome.

Other than the above I will elaborate on the changes that have been added in this PR right where they have been added.

OOH Report needless disables would be good! Sometimes somebody's disabled a line because of a reason, then the line itself gets rewritten and wouldn't fail linter anymore, but the disable is still there. We wouldn't need the disable there, but it doesn't tell us that without this set. This is a good one nesa, put these in!

For the hue notation stuff, if we see new rules but don't need them, you add them in and turn them off specifically. So we know that we didn't miss them, we specifically don't want them.

@francox9
Copy link

Looking good 💯

Copy link

@karunabaral karunabaral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. And thank you for the detailed explanation on each new rule!

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
…t now that is in dependencies and I am just gonna try if this deletion works
Copy link
Contributor

@seankoji seankoji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o/

@nesamouzehkesh nesamouzehkesh merged commit 276ef78 into master Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants