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

Added new routes #4

Merged
merged 9 commits into from
Sep 5, 2018
Merged

Conversation

prytoegrian
Copy link
Contributor

Cf. #1

Hi ! Following #1, I added some new routes. They are neither very useful nor interesting, but I hope they could entertain someone. Now, each QuoteType has a score and a uuid for sort and selection purpose.
I added :

  • /one/{uuid}
  • /top
  • /one/{uuid}/like
  • /one/{uuid}/dislike

The routes /one/{uuid}/like and /one/{uuid}/dislike don't follow ReST best practices since this principle forbid verbs in routes but I failed to find a way to represent what I wanted to build. If you have any idea, I'll take it with pleasure.

If, for a reason or another, you don't approve this PR, I won't mind. I really enjoyed play with your software and practice my golang.

Regards.

@bruno-chavez bruno-chavez merged commit e3e3f66 into bruno-chavez:master Sep 5, 2018
@bruno-chavez
Copy link
Owner

bruno-chavez commented Sep 5, 2018

Hey I just tested this fork, this is really cool, thanks for the PR!

@prytoegrian
Copy link
Contributor Author

You're welcome, thanks to you !

@prytoegrian prytoegrian deleted the pry/uuid branch September 5, 2018 06:09
@prytoegrian
Copy link
Contributor Author

Hum, I just saw you reverted the PR (I wanna use master to add some tests). Did you find any bugs in my code ?

@bruno-chavez
Copy link
Owner

bruno-chavez commented Sep 5, 2018

I reverted the PR on master since I wanted to re-structure the project to better accommodate your changes, until then I made a separate branch with all your changes.

@prytoegrian prytoegrian mentioned this pull request Sep 9, 2018
@bruno-chavez
Copy link
Owner

@prytoegrian just pushed to main all your changes, made some minor fixes, thanks for your time!

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.

2 participants