-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Use ESLint to enforce code quality + styling #5635
Conversation
I'm sorry, @vinyldarkscratch, but I am having a hard time seeing this getting merged. I'm going to close this, but I have some ideas for next steps. The real problem is that #5252 is under specified. I think it's likely there's a consensus for using ESLint in general, but the configuration and plugins and dependencies needs a complete proposal to grapple with, or needs to be broken down into smaller pieces that we can tackle one-by-one (e.g., whether to prefer arrow functions). Looking at this PR, I see a lot of changes, but I don't really understand (or know how I would understand) how every piece is necessary to satisfy #5252. To get this closer to resolution, I think we need to go back to #5252 and spell out the expectations more clearly. What code quality problems do we have in BCD? Does ESLint find those problems (either by default, or with additional config)? And what would fixing those problems entail? Then we can have a process we can work through, and we'll be able to have PRs with a clear basis for reviewing them. I know there are some other PRs that relate to this and closing this likely impacts them. I'll follow up on those PRs known to me shortly. Again, I'm sorry to be closing this. I know you've put some real effort into this and I'm appreciative of what you've done. I imagine what you've learned here will help us get to the point of eventually being able to merge something like this, but there's going to be a bit of a learning process first. Thank you. |
I totally understand, no worries! So to summarize, we should proceed with some more discussion first, before looking at getting a PR going? (I'll also be keeping this branch around in my own fork, so that it can be quickly adjusted at any time based upon what we discuss!) |
This PR introduces ESLint to help enforce code quality. Based upon its use in Stumptown, we had verbally discussed (at All Hands 2020) implementing ESLint into BCD. ESLint can integrate with Prettier as well, so we don't need to run Prettier separately -- it's all integrated right into ESLint's command. Fixes #5252.
The ESLint configuration is set to extend the default recommendations from ESLint, with some slight alterations to adhere to our code styling, such as single quotes, allowed use of the console, always end lines with semicolons, etc. As an additional feature, ESLint is also configured to enforce JSDoc comment linting.
Note: this PR will fail for now as there are many errors in our existing code to fix. I plan to follow up with modifications that fix these issues later on, after all other
linter
andinfra
PRs are merged or closed. This is marked as "not ready" until there are no issues.