Skip to content

rizan-ibrahim-node.js-w1 #7

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rizan-ibrahim
Copy link

No description provided.

Copy link

@eidosam eidosam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, Rizan! You did very well in this assignment, here are some good points:

  • Using ES Modules is good and a modern practice.
  • Proper error handling and using the correct HTTP code.
  • Defining the port as a constant is good practice.

One area for improvement is adding a description to the PR to provide more context.

Keep up the good work!

{
"name": "hackyourtemperature",
"version": "1.0.0",
"main": "index.js",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main field in package.json should match the actual entry point of the application, which is server.js in this case. This tells Node.js where to start the application when using commands like node . or npm start.

"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good practice to include the author name and public email in the author field, like this: Name <email-address>, though it's not mandatory.

@eidosam eidosam added Approved Week 1 Week 1 assignment and removed Review in progress labels Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Week 1 Week 1 assignment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants