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
[CP_24] - Add eslint spell-checker #329
Conversation
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.
@LaViro I worked on backporting this change myself. I was about to open PR, when I saw yours :D
package.json
Outdated
@@ -5,6 +5,7 @@ | |||
"main": "index.js", | |||
"scripts": { | |||
"lint": "./node_modules/.bin/eslint -c .eslintrc webpack/", | |||
"lint:spelling": "eslint ./", |
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.
Since in this branch we still use eslint
, npm run lint:spelling
is effectively the same as npm run lint
(except it might use global eslint
command, I guess).
This also means .github/workflows/js_tests.yml
should be back to run: npm run lint
, there's single command doing both things.
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.
Awesome thanks !
@@ -4,7 +4,7 @@ export const withCardsDecorator = storyFn => ( | |||
<div | |||
style={{ | |||
width: '100%', | |||
height: '100vh', | |||
height: '100%', |
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.
I encountered same problem in my backport. Not sure why spellcheck doesn't like this string, but I solved it by adding "vh" to list of ignored words.
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.
yeah, I was about to add it, although on a second thought it means the same in this case :)
@mirekdlugosz Updated and JS tests are green :) |
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.
@LaViro looks great! Thanks for taking care of backporting this.
Do you want to also backport/cherry-pick into foreman-1.22 branch?
Merged, thanks @LaViro and @mirekdlugosz! Now for foreman_22 version :) |
* Run spell-checker over JavaScript files for each PR (theforeman#327) * Add eslint spell-checker plugin and npm command to run it * Fix typo in chart id: "donunt" -> "donut" (cherry picked from commit e06b521) Conflicts: .eslintrc package.json * Fix typo in hosts sync toast (cherry picked from commit 3839714) * Fix eslint spelling error in story styles * Refactor lint script Co-authored-by: Mirek Długosz <mzalewsk@redhat.com> (cherry picked from commit c5992a6)
* Run spell-checker over JavaScript files for each PR (#327) * Add eslint spell-checker plugin and npm command to run it * Fix typo in chart id: "donunt" -> "donut" (cherry picked from commit e06b521) Conflicts: .eslintrc package.json * Fix typo in hosts sync toast (cherry picked from commit 3839714) * Fix eslint spelling error in story styles * Refactor lint script Co-authored-by: Mirek Długosz <mzalewsk@redhat.com> (cherry picked from commit c5992a6)
No description provided.