Skip to content
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

Nicole & Jess' Shipping API #3

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Nicole & Jess' Shipping API #3

wants to merge 24 commits into from

Conversation

jweeber
Copy link

@jweeber jweeber commented May 27, 2016

Pair project by Nicole and Jess.

This was hard! But we learned a lot.

Original project and PR for shipping service - sophiabaldonado/betsy#1
Shipping API - http://shipping-traverse.herokuapp.com
Betsy Traverse - http://traverse-shipping.herokuapp.com

@hougland
Copy link

hougland commented Jun 7, 2016

Hey there! Me and @TamarHershi will be reviewing your project. Woo hoo!

Right off the bat, I'm loling at " Finally got a UPS and USPS account. Only took 24 fucking hours" ha ha ha ha

//= require jquery
//= require jquery_ujs
//= require turbolinks
//= require_tree .
Copy link

Choose a reason for hiding this comment

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

Remember to delete these files you don't use. Otherwise your code reviewers have to scroll through them :(

render json: estimate.as_json, :status => :ok
rescue ActiveShipping::ResponseError
render json: [], :status => :bad_request
end
Copy link

Choose a reason for hiding this comment

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

begin/rescue/end blocks are fun!

Copy link

Choose a reason for hiding this comment

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

very clean method, yay! 🎉

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! Charles helped us out with this one, so it's still kinda confusing to me.

@hougland
Copy link

hougland commented Jun 7, 2016

Overall, great job! You made it look easy 🌟

@jweeber
Copy link
Author

jweeber commented Jun 13, 2016

Thanks for reviewing everything! 🤗

@nicosaki
Copy link

Thank you for the feedback!!

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.

4 participants