Skip to content

Cedar Katie's Task-List-API#69

Open
katiediaz wants to merge 7 commits intoAda-C16:masterfrom
katiediaz:master
Open

Cedar Katie's Task-List-API#69
katiediaz wants to merge 7 commits intoAda-C16:masterfrom
katiediaz:master

Conversation

@katiediaz
Copy link

No description provided.

Copy link

@kaidamasaki kaidamasaki 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!

I left a couple of style notes but overall everything looks good! Well done!

Comment on lines +108 to +121
# @task_bp.route("/<task_id>", methods=["GET"])
# def handle_task(task_id):
# task_id = int(task_id)
# task = Task.query.get(task_id)
# if not task:
# return make_response(f"Task {task_id} Bad data", 400)

# if request.method == GET

# for task in tasks:
# if task.id == task_id:
# return vars(task)

# return "Not found", 404

Choose a reason for hiding this comment

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

Style: Clean up commented out code.

try:
int(number)
except:
abort(make_response({"error": f"{parameter_type} must be an int"})), 400

Choose a reason for hiding this comment

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

I like that you included a parameter name in the message!

goal_id = int(goal_id)
goal = Goal.query.get(goal_id)
if goal is None:
return make_response("", 404)

Choose a reason for hiding this comment

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

It's helpful to provide detailed error messages for debugging:

Suggested change
return make_response("", 404)
return make_response({"error": f"Could not find goal #{goal_id}"}, 404)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants