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

chore(v2): add stylelint #2361

Merged
merged 3 commits into from
Mar 6, 2020
Merged

chore(v2): add stylelint #2361

merged 3 commits into from
Mar 6, 2020

Conversation

scottilee
Copy link
Contributor

Motivation

  1. Adds stylelint
  2. Adds a custom stylelint plugin to check css files for a copyright header comment
  3. Integrates stylelint into GitHub CI checks
  4. Adds and integrates stylelint into FB template

Fixes #2321

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

yarn lint

You can also remove the word "copyright" from an existing CSS file and check that yarn lint fails.

Related PRs

N/A

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 6, 2020
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit 583cb9b

https://deploy-preview-2361--docusaurus-2.netlify.com

@yangshun yangshun changed the title Incorporate stylelint into Docusaurus chore(v2): incorporate stylelint Mar 6, 2020
@yangshun yangshun changed the title chore(v2): incorporate stylelint chore(v2): add stylelint Mar 6, 2020
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Thanks! This looks great! I'll merge it first and make some tweaks later on as it's easier for me to do so. This is hugely helpful!

@@ -0,0 +1,11 @@
{
"name": "stylelint-copyright",
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is off here. I recommend you use VS Code and install the Prettier plugin to autoformat your JS files upon save.

@@ -1,3 +1,10 @@
/**
Copy link
Contributor

@yangshun yangshun Mar 6, 2020

Choose a reason for hiding this comment

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

We don't really want to add this copyright notice to the non-FB templates because they are... non-FB. It's ok I'll remove it later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know, thanks!

{
"plugins": ["./packages/stylelint-copyright/index.js"],
"rules": {
"plugin/stylelint-copyright": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is off...

@yangshun yangshun merged commit ae78c1e into facebook:master Mar 6, 2020
@@ -0,0 +1,6 @@
{
"plugins": ["../../..//stylelint-copyright/index.js"],
Copy link
Contributor

@yangshun yangshun Mar 6, 2020

Choose a reason for hiding this comment

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

Relative paths won't work because when someone initializes this template in their own machine there's no such path existing. It only works within the repo. We should publish that plugin on npm, and then we can install in package.json and import it here.

I will take care of publishing that on npm.

@@ -0,0 +1,6 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

In general I prefer to use configs in JS because you can add comments to them. But then you will need a JS parser to read the file whereas other languages can easily read JSON. Since I don't expect our stylelint config to be read by non-JS environments, we can use JS instead.

@@ -17,7 +17,8 @@
"@docusaurus/preset-classic": "^2.0.0-alpha.43",
"classnames": "^2.2.6",
"react": "^16.8.4",
"react-dom": "^16.8.4"
"react-dom": "^16.8.4",
"stylelint": "^13.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in devDependencies. dependencies are things that are needed to be bundled in the application.

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

Successfully merging this pull request may close these issues.

Add stylelint for linting CSS files about copyright headers
4 participants