Skip to content

Zoisite Say R. & Izzy B.#18

Open
LRyder17 wants to merge 27 commits into
Ada-C19:mainfrom
LRyder17:main
Open

Zoisite Say R. & Izzy B.#18
LRyder17 wants to merge 27 commits into
Ada-C19:mainfrom
LRyder17:main

Conversation

@LRyder17

Copy link
Copy Markdown

No description provided.

@kelsey-steven-ada kelsey-steven-ada left a comment

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 work on waves 1 and 2 for Solar System, Say & Izzy! It looks like you've got a handle on creating a blueprint, registering it with your Flask app, and writing routes. I like the code refactors so that the routes can stay short and sweet!

Comment thread app/routes.py Outdated
Comment on lines +30 to +33
result_list = []

for planet in planets:
result_list.append(planet.planet_to_dict())

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 would be a good candidate for a list comprehension, what could that look like?

Comment thread app/routes.py Outdated
Comment on lines +49 to +52
@planets_bp.route("/<id>", methods=["GET"])
def handle_planet(id):
planet = validate_planet(id)
return planet.planet_to_dict()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's not a rule and the code still executes just fine, but usually we'd see all the routes grouped together and all the helpers methods grouped together in a file.

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