-
-
Notifications
You must be signed in to change notification settings - Fork 69
LONDON-JAN-25 | ANDREI FILIPPOV | Module-Data Flows | WEEK 2 Feature/book-library #163
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
base: main
Are you sure you want to change the base?
Conversation
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.
You fixed all of the problems outlined in the requirements. It works well now. However, there are a few more changes you implement to make your code even better:
-
The browser tab currently shows '127.0.0.1:555/debugging/book-library. Can you update it to be more explanatory to the user. If you want to read more about this here is a doc: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/title
-
Can you review all of the places that you declared a variable with 'let'? In general, you will mostly always use const (and this should be your default). If you need to reassign a value to the variable, then you would use let. Here is a video you can watch: https://youtu.be/RE6qf3As-XU?si=_YaiEPeBC5Cx_j56
-
There are a few places where the variable name was not clear to me. For example, using 'but' to denote a button might be confusing to someone reviewing your code. Can you replace with a better name where you used 'but'? 🙂.
-
Should the code-reading/readme.md file be included in this PR?
Thank you for the comments.
So I completed the exercise to help improve my understanding. |
…dule-Data-Flows into feature/book-library
Learners, PR Template
Self checklist
Changelist
fixed bug in book library app
Questions
no questions