Skip to content

Solar System: Part 1 (Gwen and Virginia)#22

Open
virginiarare wants to merge 12 commits intoAda-C19:mainfrom
virginiarare:main
Open

Solar System: Part 1 (Gwen and Virginia)#22
virginiarare wants to merge 12 commits intoAda-C19:mainfrom
virginiarare:main

Conversation

@virginiarare
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@apradoada apradoada 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! Just one small thing you could improve upon but overall well done!

Comment thread app/__init__.py
from .routes import planet_bp
app.register_blueprint(planet_bp)

return app
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks good!

Comment thread app/routes.py Outdated

]

planet_bp = Blueprint("planet_bp", __name__, url_prefix="/planets")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job setting up the planet class, the planet list and the planet blueprint!

Comment thread app/routes.py Outdated
"name" : planet.name,
"description" : planet.description,
"distance" : planet.distance
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a really good start! One thing you could do to make this just a little more streamlined would be to create a method in your planet class that converts the planet to a dictionary! Then you can just call the planet and return its dictionary form!

Comment thread app/routes.py Outdated
if planet.id == planet_id:
return planet.dictionary

abort(make_response({"message":f"planet {planet_id} not found"}, 404)) No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The rest of this looks really great!

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.

3 participants