Set Up Automatic Code Checks for a Conduit Project#67
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on setting up the project structure and implementing the tests! The page objects are well-written. However, I'm requesting a few changes because the ESLint and Prettier configurations are currently conflicting with each other, which prevents them from working as intended [CHECKLIST ITEM #4].
To fix this, please make the following adjustments in eslint.config.mjs:
- Move
eslintConfigPrettierto be the very last element in the exported array. This ensures it can correctly override other style rules. - Remove the
max-lenrule, as code formatting should be handled exclusively by Prettier. - Remove the line that ignores
playwright.config.jsso it can be linted correctly.
Once these changes are made, the tools should work together seamlessly, and you won't need manual workarounds like the string concatenation in tests/signUp/signUpNegative.spec.js. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| /** @type {import('eslint').Linter.Config[]} */ | ||
| export default [ | ||
| { languageOptions: { globals: globals.node } }, | ||
| eslintConfigPrettier, |
There was a problem hiding this comment.
To ensure Prettier can correctly disable conflicting style rules from other configurations, eslintConfigPrettier should be the very last element in this exported array.
| 'max-len': [ | ||
| 'error', | ||
| { | ||
| code: 80, | ||
| comments: 80, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The max-len rule is responsible for code formatting, which is a task that should be exclusively handled by Prettier in this setup. Including this rule here can lead to conflicts between ESLint and Prettier. Please remove the max-len rule.
| }, | ||
| ignores: [ | ||
| '**/node_modules/*', | ||
| 'playwright.config.js', |
There was a problem hiding this comment.
The Playwright configuration is an important project file and should be included in linting. It's likely being ignored because of the max-len rule above. Once you remove the max-len rule, you should also remove this line to ensure the configuration file is linted.
| `username:Username must start with a letter, have no spaces, ` + | ||
| `and be 2 - 40 characters.`, |
There was a problem hiding this comment.
This manual string concatenation suggests a conflict between ESLint and Prettier. Your Prettier setup should handle formatting for long lines automatically, and you shouldn't need to manually break up strings to satisfy the max-len ESLint rule. This issue stems from the ESLint configuration and should be resolved there to let the tools work together correctly.
|
Changes completed |
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this pull request! I am approving your changes. You've successfully addressed all the feedback from the previous review, and the ESLint and Prettier configuration is now set up perfectly. This creates a solid foundation for maintaining code quality. The tests are well-structured, the page objects are clean, and the project now fully meets all requirements. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.