-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
Add ESLint #3152
base: main
Are you sure you want to change the base?
Add ESLint #3152
Conversation
add eslint to src project
ESLint is already included in H5BP in the project root... however, it's the older v8 release. We would appreciate a PR updating to ESlint v9. For this change the existing I don't think we need to include it in the src/, dist/ directories. We'll leave that up to users how to handle that. |
I discussed this with @roblarsen. He supported this idea |
Yeah, I didn't mind the idea of shipping it with the project, depending on what it looks like. |
@roblarsen @coliff finalization and merge or reject? |
I haven't looked at it yet. I'll look at it when I get a chance |
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.
Key Changes and Benefits:
- Addition of Linting Step:
o The new code introduces a linting step with ESLint via the lint script: "lint": "eslint ./js --fix". This ensures code quality by automatically fixing linting issues in the JavaScript files.
o The build script now also runs the linting step before executing the webpack build process: "build": "npm run lint && webpack --config webpack.config.prod.js". This is a great addition to catch potential code quality issues before building the project. - DevDependency Updates:
o ESLint Integration:
You've added eslint, along with @eslint/js and globals as devDependencies. This enhances the development workflow by enforcing code standards and identifying possible issues early in the development process.
o Version Locking:
It's great to see version locking for dependencies like eslint and globals, which ensures stability in your development environment. - Overall Workflow Improvement:
o By including the linting step as part of the build process, you're ensuring that only high-quality, properly formatted code makes it to production. This reduces the risk of bugs and increases maintainability.
Suggestions for Further Improvement:
• Unit Testing Setup:
o Since the test script still outputs "Error: no test specified", consider adding a unit testing framework (like Mocha, Jest, or Jasmine) to further improve your code quality and ensure functionality.
• ESLint Configuration:
o Ensure that your .eslintrc or equivalent configuration is well-tuned for your project’s coding standards. You might want to review the rules to align them with your team's style guide.
@roblarsen just a reminder |
Types of changes
Checklist: