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

Shipping Service #5

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

Shipping Service #5

wants to merge 17 commits into from

Conversation

rishallen
Copy link

Yordanos and Risha

This was a challenging and yet very amazing project. It was interesting to see how programs can interact with each other.

@dezshino
Copy link

dezshino commented Jun 7, 2016

Hi! Will you provide a link to your betsy code? :) &if it's deployed to heroku those links as well

@@ -0,0 +1,28 @@
== README
Copy link

Choose a reason for hiding this comment

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

I recommend updating the rails generated readme with info about your project (even a copy paste of the project requirements), it's helpful for if you look back over projects later on

@emgord
Copy link

emgord commented Jun 13, 2016

Really nice job with this project! I remember this being one of the most challenging projects for me. You did a really great job separating logic to calculate shipping cost on your model with lots of great helper methods and keeping your controller focused on sending information back through the API.
This isn't your fault but somehow your pull request is showing over 5000 changed files which made it hard to review and find the files that you actually changed, so I apologize if I missed anything. It looks like maybe a bunch of the code for the gems is showing up for this pull request? I'd also love to see your API in action and integrated with the betsy project, I didn't have the info for that pull request to review.

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