-
Notifications
You must be signed in to change notification settings - Fork 134
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: format and lint with ESLint and Prettier #172
Conversation
@@ -0,0 +1 @@ | |||
* text=auto eol=lf |
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.
As recommended per https://prettier.io/docs/en/options.html#end-of-line
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'd prefer to limit prettier
to the JS/TS files for now, to remain consistent with other repos. See the comment about semicolons in README.md
, and generally I think it's nice to avoid unnecessary drift between boilerplate config and CI that we use across the ecosystem - makes it easier to find actual drift in the future.
README.md
Outdated
const { updateElectronApp } = require('update-electron-app') | ||
updateElectronApp() | ||
const { updateElectronApp } = require('update-electron-app'); | ||
updateElectronApp(); |
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.
We don't include semicolons in examples in end user code, I'd prefer we didn't add these so that we remain consistent across our repos.
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 think this used to be linted by standard-markdown
. I've added prettier-ignore
statements in accordance, but this is unlinted now. I can add the other dependency back in if 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.
If we want Markdown linting in the future, for consistency we should use @electron/lint-roller
.
Co-authored-by: David Sanders <[email protected]>
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Stacks on top of #171 because that PR addresses some of the lint problems we have on
main
.We previously added a Prettier configuration in 107661d but never configured it to run via NPM script nor in CI.
This PR addresses that and replaces the
standard
andstandard-markdown
packages witheslint
andtypescript-eslint
. It also bumps uptypescript
to the latest stable version.Update: