Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a great attempt, and the core logic for building the table is correct. However, this requires changes because there are a few critical issues in src/scripts/main.js that cause syntax errors and prevent the code from running.
- Conflicting Variable: You have a large, hardcoded
peoplearray at the top of the file, which conflicts with theimport people from './people.json';statement later on. Please remove the hardcoded array entirely. - Misplaced
import: Allimportstatements must be at the very top of a JavaScript module. Please move theimportforpeople.jsonto the first line of the file.
Fixing these two issues should get your script working correctly. Well done on the implementation of the table generation logic!
✨ 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
src/scripts/main.js
Outdated
|
|
||
| // eslint-disable-next-line no-console | ||
| console.log(people); // you can remove it | ||
| 'use strict'; |
There was a problem hiding this comment.
The 'use strict' directive is only effective when placed at the very beginning of a script or function. Because there's a large block of code before this line, this directive has no effect. Also, JavaScript modules (files using import) are automatically in strict mode, making this line redundant.
| console.log(people); // you can remove it | ||
| 'use strict'; | ||
|
|
||
| import people from './people.json'; |
There was a problem hiding this comment.
While importing from people.json is the correct approach, this statement is misplaced. All import declarations must be at the very top of the file. More critically, there's a people constant declared at the beginning of the file (lines 3-355) that conflicts with this import, which will cause a syntax error. The hardcoded array should be removed, and this import statement should be moved to the top of the file.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This requires a few changes as the code currently has critical syntax errors that prevent it from running and cause the build to fail. The logic for building the table is good, but we need to fix the file setup.
Here are the key issues to address in src/scripts/main.js:
- Duplicate Variable Declaration: The
peopleconstant is declared twice: once as a large hardcoded array and again via theimportstatement. This causes the build to fail with the errorIdentifier 'people' has already been declared. Please remove the entire hardcoded array. - Misplaced
importStatement: Allimportstatements must be at the very top of a JavaScript module. Please moveimport people from './people.json';to the top of the file. - Incorrect Property Name: The code attempts to access
person.gender, but the property in the data is namedsex. This should be updated toperson.sexto display the correct information.
Fixing these items will allow your script to run and pass the checks.
✨ 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
|
|
||
| // eslint-disable-next-line no-console | ||
| console.log(people); // you can remove it | ||
| import people from './people.json'; |
There was a problem hiding this comment.
As mentioned in the previous review, import statements must be at the very top of a JavaScript module, right after the 'use strict'; directive.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this update! I am approving your solution as you've done a fantastic job addressing all the previous feedback. The script now runs perfectly without any errors. Your code is clean, efficient, and correctly implements all the requirements, from calculating the age and century to dynamically generating the table rows. This is a great example of a well-executed solution. Keep up the fantastic work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.