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

Jade and Mindy - Shipping-Service #6

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

Conversation

mcarson1111
Copy link

@mcarson1111 mcarson1111 commented May 27, 2016

This was a very challenging project; we learned how to read through someone else's code, build our own API and work with new API's that generally are known for having internal problems ( as per internet results).

I'm really proud of how well we communicated throughout the project, and were able to work through errors while building the connection to the API, and working on different parts of the app simultaneously. We got some really strange errors with the .gitignore file, the api responses and some challenging errors with code that wasn't originally ours. As of now, it's mostly working, and we learned a lot, so that's great ^_^

links: http://suchhipsterly.herokuapp.com/
http://super-sweet-shippers.herokuapp.com/

… Tested it all out in rails console and everything works, yaaay
Why is this showing :( :( :( It's ignored but still here booooo
@rileslovesyall
Copy link

Hey y'all, did you deploy this to Heroku? If so, could you provide the link for that?

fedex_rates = get_rates(fedex)

request = [ups, fedex].as_json
response = [ups_rates, fedex_rates].as_json

Choose a reason for hiding this comment

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

Did you end up not using usps? If it's not being used in your code, maybe you could comment it out and explain why you wanted to keep it around for future use, or just remove it before the PR?

@rileslovesyall
Copy link

Also, is there a PR for the Betsy part of the project? I looked under Hipsterly, and I only see one PR from the other group.

class Carrier < ActiveRecord::Base
validates :request, presence: true
validates :response, presence: true

Choose a reason for hiding this comment

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

You could make this one line by doing validates_presence_of :request, :response if you wanted. Just fyi.

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