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

Change grape dependency to support new versions #20

Merged
merged 1 commit into from
Nov 19, 2013

Conversation

cheef
Copy link
Contributor

@cheef cheef commented Nov 19, 2013

New grape version working fine for me with this gem

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.13%) when pulling ef5682a on cheef:master into f16bfb2 on LTe:master.

@@ -15,3 +15,4 @@ spec/reports
test/tmp
test/version_tmp
tmp
.idea
Copy link
Member

Choose a reason for hiding this comment

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

What's .idea? This should probably not be part of this commit, it's not relevant.

@dblock
Copy link
Member

dblock commented Nov 19, 2013

Add a Next Release section to CHANGELOG and a 1-liner that says something like "Relaxed dependency on a specific version of Grape", please.

Make these changes via git commit --amend, please.

@dblock
Copy link
Member

dblock commented Nov 19, 2013

One more thing, the README tells you to use a specific version of things, would you mind fixing that, too? Appreciate it.

@cheef
Copy link
Contributor Author

cheef commented Nov 19, 2013

I think it's better to change vesion in README too, agree?

@dblock
Copy link
Member

dblock commented Nov 19, 2013

I mean we say gem 'grape', "~> 0.2.3", it should just say gem 'grape' probably.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.13%) when pulling 4673aa8 on cheef:master into f16bfb2 on LTe:master.

@cheef
Copy link
Contributor Author

cheef commented Nov 19, 2013

Added all changes

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.13%) when pulling c393331 on cheef:master into f16bfb2 on LTe:master.

@@ -1,3 +1,7 @@
#### Next Release

* Relaxed dependency on a specific version of Grape
Copy link
Member

Choose a reason for hiding this comment

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

Can you please put this in the same format as all the other changes below? Thx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, did this. And omitted commit link, seems it doesn't make sense until pull request not merged.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.48%) when pulling 8ac20a0 on cheef:master into f16bfb2 on LTe:master.

@@ -1,3 +1,7 @@
#### Next Release

* Relaxed dependency on a specific version of Grape [#20](https://github.com/LTe/grape-rabl/pull/11) [@cheef](https://github.com/cheef)
Copy link
Member

Choose a reason for hiding this comment

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

Link in CHANGELOG should pointed to #20 instead of #11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and this is correct url to my pull request.

Copy link
Member

Choose a reason for hiding this comment

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

That link, /11 should say /20, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, not noticed

@coveralls
Copy link

Coverage Status

Coverage increased (+0.48%) when pulling 56da0a5 on cheef:master into f16bfb2 on LTe:master.

@dblock
Copy link
Member

dblock commented Nov 19, 2013

Merging, thanks for being patient with the nitpicking.

dblock added a commit that referenced this pull request Nov 19, 2013
Change grape dependency to support new versions
@dblock dblock merged commit bfb6e66 into ruby-grape:master Nov 19, 2013
@dblock
Copy link
Member

dblock commented Nov 19, 2013

0.2.2 released

@LTe
Copy link
Member

LTe commented Nov 19, 2013

@cheef thank for this pull request. You are awesome 👍
@dblock thank for release! ❤️

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.

None yet

4 participants