Develop #3445
Develop #3445
Conversation
Feature/prettier
There was a problem hiding this comment.
Thank you for your hard work on this project! There are a few critical issues that need to be addressed before we can approve the pull request. The env configuration in your ESLint file specifies es2024, which is not a valid ECMAScript version and could lead to linting errors. Please update it to a supported version like es2021 or es2022. Additionally, consider updating the actions/setup-node version to v3 for better stability and support. Once these changes are made, your code will be in great shape! Keep up the great work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
.eslintrc.cjs
Outdated
| module.exports = { | ||
| env: { | ||
| browser: true, | ||
| es2024: true, |
There was a problem hiding this comment.
The env property specifies es2024, which is not a valid ECMAScript version for ESLint. Consider using a supported version like es2021 or es2022.
| @@ -11,19 +11,21 @@ jobs: | |||
|
|
|||
| strategy: | |||
| matrix: | |||
| node-version: [14.x] | |||
| node-version: [20.x] | |||
|
|
|||
| steps: | |||
| - uses: actions/checkout@v2 | |||
| - name: Use Node.js ${{ matrix.node-version }} | |||
| uses: actions/setup-node@v1 | |||
There was a problem hiding this comment.
Consider updating the actions/setup-node version to v3 for improved features and support. The latest version is generally more stable and includes the latest updates.
| # fix rollup issue | ||
| - run: rm -rf node_modules package-lock.json |
There was a problem hiding this comment.
The step to remove node_modules and package-lock.json is a workaround for a Rollup issue. Ensure this is necessary for your project, as it can increase build times and may not be needed if the underlying issue is resolved.
There was a problem hiding this comment.
Great job on your project! 🎉 Your ESLint configuration, Cypress setup, and component implementation are well-structured and meet the task requirements. However, there are a couple of issues to address:
-
GitHub Actions Workflow: Please update the
actions/setup-nodeversion fromv1tov3in bothtest.ymlandlint.ymlfiles for better stability and support. -
ESLint Configuration: You've successfully corrected the
envconfiguration toes2022, resolving the previous issue.
Your code is on the right track, and the decision to approve is to help you move forward with your learning. Please review the comments to improve your code further. Addressing these issues will enhance your project's functionality and maintainability. Keep up the great work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -11,19 +11,21 @@ jobs: | |||
|
|
|||
| strategy: | |||
| matrix: | |||
| node-version: [14.x] | |||
| node-version: [20.x] | |||
|
|
|||
| steps: | |||
| - uses: actions/checkout@v2 | |||
| - name: Use Node.js ${{ matrix.node-version }} | |||
| uses: actions/setup-node@v1 | |||
There was a problem hiding this comment.
Consider updating actions/setup-node@v1 to actions/setup-node@v3 for improved stability and support.
DEMO LINK