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

Updating stylelint from v13 to v14 and fixing linting issues WIP #14743

Closed
wants to merge 1 commit into from

Conversation

georgewrmarshall
Copy link
Contributor

@georgewrmarshall georgewrmarshall commented May 18, 2022

Explanation

Currently we are using stylelint 13.6. This PR updates stylelint to 14.6.3 by following the Migrating to 14.0.0 guide. It also updates a fair amount of css linting issues that are flagged by v14

Some issues with this PR that should be resolved before it can be reviewed

  • gulp-stylelint which we are using(not sure for what) does not support v14 this PR is currently using a workaround Upgrade to newer version of stylelint  olegskl/gulp-stylelint#132 (comment) is this a security issue and if so how do we make so it's not?
  • Fix lavamoat issues
  • Renamed all mixins to use lowercase to follow better naming convention. Stylelint actually picked up on this. Is this ok? Will it cause confusion?

vscode stylelint plugin also mentions that styleint 13 is no longer supported.

Screen Shot 2022-05-14 at 7 36 03 PM

Manual Testing Steps

Run yarn lint: styles

Pre-Merge Checklist

  • PR template is filled out
  • Manual testing complete & passed
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • IF QA attention is required, "QA Board" label has been applied
  • PR has been added to the appropriate release Milestone

@georgewrmarshall georgewrmarshall added the DO-NOT-MERGE Pull requests that should not be merged label May 18, 2022
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@@ -100,12 +101,15 @@ module.exports = {
'selector-attribute-brackets-space-inside': 'never',
'selector-attribute-operator-space-after': 'never',
'selector-attribute-operator-space-before': 'never',
'selector-class-pattern':
'^(?:(?:o|c|u|t|s|is|has|_|js|qa)-)?[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*(?:__[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*)?(?:--[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*)?(?:\\[.+\\])?$', // BEM naming convention
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an attempt at checking for the BEM naming convention. It's works quite well but we do have css like:

.component-name {
  _sub-component {
    &.btn {
       width: 100%;
    }
  }
}

which it flags

@georgewrmarshall
Copy link
Contributor Author

Closing in favour of #13896

@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DO-NOT-MERGE Pull requests that should not be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant