-
Notifications
You must be signed in to change notification settings - Fork 622
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
frontend: apply prettier to cypress/webapp #417
Conversation
489890d
to
44ac4a7
Compare
Codecov Report
@@ Coverage Diff @@
## main #417 +/- ##
=======================================
Coverage 50.85% 50.85%
=======================================
Files 125 125
Lines 5710 5710
=======================================
Hits 2903 2903
Misses 2528 2528
Partials 279 279 Continue to review full report at Codecov.
|
ParametersDetails
Result
Details
Screenshots |
idea here is to run eslint + prettier against staged files so that nothing broken is pushed frontend: add cypress eslint plugin frontend: reformat more files
@@ -1,13 +1,13 @@ | |||
import { BAR_HEIGHT } from '../../webapp/javascript/components/FlameGraph/FlameGraphComponent/index.jsx' | |||
import { BAR_HEIGHT } from '../../webapp/javascript/components/FlameGraph/FlameGraphComponent'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this cause a problem or is the /index.jsx
not needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an eslint
error
Unexpected use of file extension "jsx" for "../../webapp/javascript/components/FlameGraph/FlameGraphComponent/index.jsx" import/extensions
We weren't catching it before because our lint rule was only applied to js/jsx
files (this one is a ts
file).
But overall it's not needed, AFAIK all tools follow the convention that foo/index.{js,jsx,ts,tsx}
equals to /foo
retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eh-am Thank you for doing this, this is great!
Ran
eslint
andprettier
against ALL files. That is so that everything is consistent (from js files to yamls)This ends up breaking
git blame
(since I modified all files).To solve that added a
postinstall
script that runsgit config blame.ignoreRevsFile .git-blame-ignore-revs
, which makesgit blame
ignore commits from the.git-blame-ignore-revs
file.Therefore, we require existing code to run
yarn
again.Plus added
husky
andlint-staged
integration. Which makes us runeslint
andprettier
to staged files when committing. That will guarantee all new modifications are formatted.Also added a ci check to validate files have been formatted (let's say the husky hook didn't work for some reason).