-
Notifications
You must be signed in to change notification settings - Fork 7
Yahya Al-Ademi-w1-Node.js #1
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?
Yahya Al-Ademi-w1-Node.js #1
Conversation
durw4rd
left a comment
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.
Excellent work Yahia, more than what the instructions asked for! I appreciate that you pass back the response codes, verify the payload in your callback function.
There were a couple of small things, such as using boolean values instead of strings for true/false values, and consistent naming of the keys in the JSON object, but good job overall.
| const { cityName } = req.body; | ||
| if (!cityName) { | ||
| return res.status(400).json({ | ||
| success: "false", |
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.
SUGGESTION: Use boolean true instead of string here.
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.
Thanks a lot for the helpful feedback! I’ll make sure to use booleans instead of strings.
| }); | ||
| } | ||
| res.status(200).json({ | ||
| success: "true", |
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.
SUGGESTION: Use boolean true instead of string here.
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.
Thanks a lot for the helpful feedback! I’ll make sure to use booleans instead of strings.
| Data: { | ||
| City: `You entered: ${cityName}`, |
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.
SUGGESTION: Use lowercase 'data' and 'city' for consistency.
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.
I will keep the JSON keys consistent next time. And I’ll keep improving on those details. thank you so much
No description provided.