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

Document recommended eslint/prettier workflow #1474

Closed
miketaylr opened this issue Apr 3, 2017 · 12 comments
Closed

Document recommended eslint/prettier workflow #1474

miketaylr opened this issue Apr 3, 2017 · 12 comments

Comments

@miketaylr
Copy link
Member

Support for prettier landed in #1468.

We should determine a simple workflow, and document it so we're all on the same page.

Strawman proposal:

  1. write some code
  2. run npm run lint
  3. run npm run fix (if there were simple style errors...)
  4. commit, make PR, etc.
@magsout
Copy link
Member

magsout commented Apr 3, 2017

@miketaylr looks good 👍 it's I think the best way.

@zoepage
Copy link
Member

zoepage commented Apr 4, 2017

+1

Do we want to add a hook to lint automatically before committing?
https://github.com/prettier/prettier#pre-commit-hook-for-changed-files

@magsout
Copy link
Member

magsout commented Apr 4, 2017

@zoepage

Do we want to add a hook to lint automatically before committing?

It was my first thought and I made it in my initial PR. But after I did not know if it was good or practical to do that especially after the comment of @karlcow #1434 (comment)

@zoepage
Copy link
Member

zoepage commented Apr 4, 2017

@magsout Just to clarify, I was just talking about linting. Not fixing :)

Not a fan of automated fixing either, so I agree with @karlcow. Linting is something people like to forget (as same as running the build and wondering, why the styles are missing... Me. Always. 😁)

So basically a hook that would lint after you git add your files as precommit. So you'd be able to fix the linting by yourself or automatically, if you decide so, before you commit.

@miketaylr
Copy link
Member Author

Decision: we'd like to figure out a pre-commit style workflow. But possibly good to document manual way as well?

@magsout
Copy link
Member

magsout commented Apr 4, 2017

@zoepage

@magsout Just to clarify, I was just talking about linting. Not fixing :)

ah yes, you meant npm run lint ? not npm run fix ?

@magsout
Copy link
Member

magsout commented Apr 4, 2017

So something like:

"scripts": {
    "precommit": "lint-staged",
 },
"lint-staged": {
    "{webcompat/static/js/lib,tests}__}/**/*.js": [
      "npm run lint"
    ]
  },

@miketaylr miketaylr added the ready label Apr 4, 2017
@miketaylr
Copy link
Member Author

So with a pre-commit lint, I guess once you try to commit w/ style errors, it will fail and then it's up to you to run npm run fix? @zoepage is that what you would expect?

@zoepage
Copy link
Member

zoepage commented Apr 4, 2017

@miketaylr
The pre-commit should lint after adding the files through git add, but before the commit itself. Then you could run npm run fix` or fix manually. This is what I would expect. :)

@miketaylr
Copy link
Member Author

Cool, thanks! I can live with that. :)

@magsout want to implement what you wrote in #1474 (comment)?

@miketaylr
Copy link
Member Author

@magsout
Copy link
Member

magsout commented Apr 26, 2017

@miketaylr certainly

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

No branches or pull requests

4 participants