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

Update Readme with Mounting Multiple Versions #1349

Closed
wants to merge 2 commits into from

Conversation

przbadu
Copy link

@przbadu przbadu commented Apr 6, 2016

@przbadu
Copy link
Author

przbadu commented Apr 6, 2016

@dblock: I just locked version of grape-entity to 0.5.0 and made a PR. Your suggestion for #1350. All test case are passing in update-readme branch now...waiting for travis ci to be successful

@dblock
Copy link
Member

dblock commented Apr 6, 2016

FYI I did that on master so we can have a build in #1353.

@przbadu
Copy link
Author

przbadu commented Apr 6, 2016

Great, thank you, let me pull it and merge it

@przbadu
Copy link
Author

przbadu commented Apr 7, 2016

@dblock : please check README and let me know for further improvements.

Thanks

@dblock
Copy link
Member

dblock commented Apr 7, 2016

It doesn't quite read as documentation, more like a blog post or example. Specifically, I'm not in love with curl examples and I definitely don't think we need to say what it took to make it happen (the grape fix). There should be a way to reduce the code, too, and also needs to be rubocop-ed.

It's a good start though. I can take a stab at this later or you can try iterating on it.

@@ -25,6 +25,7 @@
- [Header](#header)
- [Accept-Version Header](#accept-version-header)
- [Param](#param)
- [Mounting Multiple Versions](#mounting-multiple-versions)
Copy link
Member

Choose a reason for hiding this comment

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

I think this belongs in Mounting more than Versioning, because Versioning is all about kinds of versioning schemes.

@dblock
Copy link
Member

dblock commented Jun 2, 2016

I appreciate the effort here, but I think it's a bit far from reading like a doc and wasn't active after my comments. I'll close it for now, please feel free to reopen with an update.

@dblock dblock closed this Jun 2, 2016
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