Skip to content

chore(vscode-settings): remove deprecated property; disable postcss validate #3325

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 28, 2024
Merged

chore(vscode-settings): remove deprecated property; disable postcss validate #3325

merged 1 commit into from
Oct 28, 2024

Conversation

@cdransf
Copy link
Member

@cdransf cdransf commented Oct 24, 2024

Description

  • Removes deprecated aslant.packageManager setting
  • Adds postcss.validate: false as postcss.validate conflicts with our stylelint and posts

How and where has this been tested?

Verified locally. Removing the following:

"at-rule-no-unknown": [
  true,
  {
    ignoreAtRules: ["extend", "container", "each", "include", "mixin"],
  },
],

Raises a lint violation against at-rule-no-unknown, establishing that ignoreAtRules is being applied (as no exceptions are now being defined in the rule definition).

Restoring the rule then returns our unknownAtRules violation which is attributed to postcss.

Adding postcss.validate: false to the workspace settings resolves the issue. Additionally — removing "extend" from ignoreAtRules and restarting the stylelint server in VS Code causes stylelint to surface a violation — we can then rely on stylelint to provide coverage for syntax violations like this.

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@cdransf cdransf added size-3 M ~18-30hrs; moderate effort or complexity, several work days needed. skip_vrt Add to a PR to skip running VRT (but still pass the action) ready-for-review labels Oct 24, 2024
@cdransf cdransf self-assigned this Oct 24, 2024
@changeset-bot
Copy link

changeset-bot bot commented Oct 24, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 58af8d4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cdransf cdransf force-pushed the cdransf/workspace-settings branch from 65ff093 to 1062688 Compare October 24, 2024 23:42
@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2024 •
edited
Loading

🚀 Deployed on https://pr-3325--spectrum-css.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2024 •
edited
Loading

File metrics

Summary

Total size: 4.30 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@cdransf cdransf force-pushed the cdransf/workspace-settings branch from 1062688 to 1b9c54c Compare October 28, 2024 16:16
castastrophe
castastrophe suggested changes Oct 28, 2024
View reviewed changes
.vscode/settings.json
"postcss": "css"
},
"eslint.format.enable": true,
"eslint.packageManager": "yarn",
Copy link
Contributor

@castastrophe castastrophe Oct 28, 2024

Choose a reason for hiding this comment

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

Why remove this?

Copy link
Member Author

@cdransf cdransf Oct 28, 2024

Choose a reason for hiding this comment

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

We can keep it — but VS Code indicated that it's now handled by default and the setting has been deprecated.

Copy link
Member Author

@cdransf cdransf Oct 28, 2024

Choose a reason for hiding this comment

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

https://github.com/microsoft/vscode-eslint/blob/fa396e45eb25087950c81c9a9f9eb6f7d1f2818f/README.md?plain=1#L218

.vscode/settings.json
}
],
"js/ts.implicitProjectConfig.experimentalDecorators": true,
"postcss.validate": false,
Copy link
Contributor

@castastrophe castastrophe Oct 28, 2024

Choose a reason for hiding this comment

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

This looks good

@cdransf cdransf force-pushed the cdransf/workspace-settings branch from 1b9c54c to 048e6e0 Compare October 28, 2024 16:46
@cdransf cdransf requested a review from castastrophe October 28, 2024 16:48
@castastrophe castastrophe force-pushed the cdransf/workspace-settings branch from 048e6e0 to 58af8d4 Compare October 28, 2024 17:09
@castastrophe castastrophe enabled auto-merge (squash) October 28, 2024 17:10
@castastrophe castastrophe merged commit 4f45494 into main Oct 28, 2024
18 of 24 checks passed
@castastrophe castastrophe deleted the cdransf/workspace-settings branch October 28, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@5t3ph 5t3ph 5t3ph approved these changes

@pfulton pfulton Awaiting requested review from pfulton

@jawinn jawinn Awaiting requested review from jawinn

+1 more reviewer

@castastrophe castastrophe castastrophe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@cdransf cdransf

Labels

ready-for-review size-3 M ~18-30hrs; moderate effort or complexity, several work days needed. skip_vrt Add to a PR to skip running VRT (but still pass the action)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cdransf @castastrophe @5t3ph