Skip to content

Khiro-W2-Node.js #20

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 4 commits into
base: main
Choose a base branch
from
Open

Conversation

khiroA
Copy link

@khiroA khiroA commented Feb 25, 2025

Completed the temperature app by fetching weather data from the openweathermap, accepting a city name via the POST method, and writing tests for the app.

@xed-euteon xed-euteon added To review Week 2 Week 2 assignment labels Mar 3, 2025
@xed-euteon xed-euteon self-assigned this Mar 3, 2025
Copy link

@xed-euteon xed-euteon left a comment

Choose a reason for hiding this comment

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

Great work,

You've made some good enough use case as required in the assignment requirements, with your clean structure you have covered key test scenarios.

One note though: you have placed your homework in the config-files lol...

Anyways, LGTM; keep it up. 🎉

Comment on lines +3 to +10
import { API_KEY } from "./sources/keys.js";
const app = express();
app.use(express.json());

app.get("/", (req, res) => {
res.send("Hello from the backend ");
});
app.post("/weather", async (req, res) => {

Choose a reason for hiding this comment

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

consistent line spacing improves readability.

Comment on lines +1 to +3
node_modules/
.env
.DS_Store

Choose a reason for hiding this comment

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

👍🏻

Comment on lines +1 to +3
import dotenv from "dotenv";
dotenv.config();
export const API_KEY = process.env.API_KEY;

Choose a reason for hiding this comment

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

🥇

Comment on lines +23 to +25
if (response.status === 404) {
return res.status(404).json({ Error: "Invalid city name" });
} else {

Choose a reason for hiding this comment

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

Two points:

  • it's a good idea to check for the response.ok as it's not always a 404 same as not okay
  • you are returning, it's an early return, no need for an else statement.

Comment on lines +6 to +9
"scripts": {
"test": "jest",
"start": "node server.js",
"dev": "nodemon server.js"

Choose a reason for hiding this comment

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

Love it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Week 2 Week 2 assignment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants