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

Remove function-calc-no-invalid #5296

Merged
merged 3 commits into from May 14, 2021
Merged

Remove function-calc-no-invalid #5296

merged 3 commits into from May 14, 2021

Conversation

jeddy3
Copy link
Member

@jeddy3 jeddy3 commented May 12, 2021

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

Closes #4731

Is there anything in the PR that needs further explanation?

Removes one of the two code scanning vulnerabilities. This one is found in the generated parser code used by the rule.

This is the 2nd pull request for the v14 branch.

Once we merged this pull request and #5295, we'll be able to tackle #5289 (removing the syntax option). Once that is done, I believe all the other 14.0.0 release issues, e.g. moving to ESM and migrating to PostCSS 8, will be unblocked and can be worked on in parallel because they shouldn't conflict with each other.

@ybiquitous
Copy link
Member

Removes one of the two code scanning vulnerabilities. This one is found in the generated parser code used by the rule.

@jeddy3 Can we remove lib/utils/parseCalcExpression also via this PR? It seems to be used only by function-calc-no-invalid.

@ybiquitous
Copy link
Member

One more question: should we announce the deprecation of function-calc-no-invalid before releasing v14.0.0?
That is, should we release some 13.x version marking this rule as deprecated?

@jeddy3
Copy link
Member Author

jeddy3 commented May 12, 2021

One more question: should we announce the deprecation of function-calc-no-invalid before releasing v14.0.0?

I wasn't going to bother with a deprecation cycle. It's needless work.

@ybiquitous
Copy link
Member

I wasn't going to bother with a deprecation cycle. It's needless work.

Make sense. Thank you for the answer. 👍🏼

@ybiquitous
Copy link
Member

Thanks for the commit f117326. Can we also remove the following lines?

$ git grep parseCalcExpression
.eslintignore:3:lib/utils/parseCalcExpression/parser.js
.prettierignore:13:lib/utils/parseCalcExpression/parser.js
package.json:78:      "!lib/utils/parseCalcExpression/parser.js"

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

LGTM! 🙌🏼

@jeddy3
Copy link
Member Author

jeddy3 commented May 12, 2021

@ybiquitous Thanks for catching all the bits I missed!

@jeddy3 jeddy3 force-pushed the remove-function-calc-no-invalid branch from e846198 to 95776fc Compare May 14, 2021 06:22
@jeddy3 jeddy3 merged commit 4b82c24 into v14 May 14, 2021
@jeddy3 jeddy3 deleted the remove-function-calc-no-invalid branch May 14, 2021 06:31
@jeddy3
Copy link
Member Author

jeddy3 commented May 14, 2021

v14 changelog:

  • Removed: function-calc-no-invalid (#5296).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants