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 Val and Sarah #9

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

Conversation

pottery123
Copy link

@pottery123 pottery123 commented May 27, 2016

What we are proud of:
Val: Realizing that we had put the whole shipping-quote architecture on the wrong page and then coming up with a clever workaround to prevent us from having to redo a ton of work

Sarah: ?

petty website
http://agile-shore-50946.herokuapp.com

API Shipping
https://evening-depths-89076.herokuapp.com/

link to petsy repo https://github.com/vconklin/petsy
for some reason we couldn't make a pull request because of the unique way we had to set up our project

digivava and others added 30 commits May 23, 2016 15:06
…ad given that code to someone on Slack so I was able to paste it back in, whoops. This commit contains that USPS test code.

def index
# adds the request to the log??
# HTTParty.post(log_path, body: params.to_json)
Copy link

Choose a reason for hiding this comment

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

Generally, it's good coding etiquette to remove any unnecessary comments before making a final PR. It's definitely fair to keep comments that help explain what is happening, like if you feel your audience needs an explanation of what a function does.

@knaydee
Copy link

knaydee commented Jun 13, 2016

I remember this project being especially challenging. High five! Good job! 👍 Too bad you weren't able to make a PR with the changes you made to betsy. I might have more feedback then :) Nice commit messages, too! I can generally work out what you did at each commit

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