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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mention rack 2.1.0 support in UPGRADING.md #2130

Merged
merged 3 commits into from May 5, 2021
Merged

Conversation

maxkerp
Copy link
Contributor

@maxkerp maxkerp commented Nov 17, 2020

I'm currently upgrading a fairly big rails app I did not write myself which uses grape. Since I unfortunately never used grape before (I want to though 馃槈), I was in unknown territory and it took me quite I while to figure out that grape added support for rack >= 2.1.0 first on 1.3.0.

First thing I did when running into [https://github.com//issues/1966](this error) was checking the Upgrade.md file for any signs of rack incompatibility, but I did not find any. I wish I did, since it took me a few hours to find the mentioned issue. (I really have to up my google-fu, I know 馃檲)

But for any poor soul in the future, who is just as bad as googling as I am, it might be helpful to mention this in the Upgrade guide :)

I'm currently upgrading a fairly big rails app I did not write myself which uses grape. Since I unfortunately never used grape before (I want to though 馃槈), I was in unknown territory and it took me quite I while to figure out that grape added support for rack >= 2.1.0 first on 1.3.0.

First thing I did when running into [ruby-grape#1966 error) was checking the Upgrade.md file for any signs of rack incompatibility, but I did not find any. I wish I did, since it took me a few hours to find the mentioned issue. (I really have to up my google-fu, I know 馃檲)

But for any poor soul in the future, who is just as bad as googling as I am, it might be helpful to mention this in the Upgrade guide :)
@grape-bot
Copy link

1 Warning
鈿狅笍 Unless you鈥檙e refactoring existing code, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#2130](https://github.com/ruby-grape/grape/pull/2130): Mention rack 2.1.0 support in upgrading.md - [@maxkerp](https://github.com/maxkerp).

Generated by 馃毇 danger

@dblock
Copy link
Member

dblock commented Nov 17, 2020

Thanks! Explain what the developer that's upgrading has to do though? Does one have to upgrade and therefore add a specific version of Rack to Gemfile? Link to the issue?

@maxkerp
Copy link
Contributor Author

maxkerp commented Nov 17, 2020

Well that got merged fast 馃榿 Always feels good to contribute to oss, even when it's a tiny bit 鈽猴笍
(Edit: Ehmm, I just realized that the PR is still open. I don't know why I thought it was already merged 馃槄 Anyway, I'm glad I can help and let me know if I can improve on this 馃槂)

I had the option to either upgrade to grape >= 1.3.0 or ensure that we are using a compatible version of rack.

Since I want to wait with the upgrade of grape til we are done with the upgrade to rails 5.0 I opted to pin the version of rack to ~> 2.0.0 so it will be below version 2.1.0.

Since rack isn't part of a Gemfile in rails project per se I needed to add it altogether like this:

gem 'rack', '~> 2.0.0'

I will remove this line from my Gemfile once I'm done upgrading to grape >= 1.3.0 :)

Does this answer your questions?

@dblock
Copy link
Member

dblock commented Nov 17, 2020

First, thanks. But I am looking for a sentence of instructions below the line that you're adding to the user UPGRADING. So I am confused to what we're trying to tell the user. UPGRADING instructions should tell users what to do, CHANGELOG says what changed and feels like we're mixing the two.

@maxkerp
Copy link
Contributor Author

maxkerp commented Nov 18, 2020

Know I understand and I think this is kind of tricky. The information I thought I might find here is the incompatibility of grape versions with other "big" gems/dependencies. When I looked at the file I wasn't looking for instructions of how I upgrade to a specific version, but rather of why I would want to.

The line I added tells me that versions <= 1.3.0 don't work with rack >= 2.1.0 and from there I can decide what to do next. This is probably very project specific. Pinning the rack version worked for me, but might not for someone else.

So if you are looking for instructions it might be something like

In the section "Upgrading to >= 1.3.0":

"You will need to upgrade to this version if you depend on rack >= 2.1.0"

Or in the section "Upgrading to >= 1.2.5":

"If something prevents you from upgrading to version >= 1.3.0 it might be helpful to pin rack to ~> 2.0.0 in your Gemfile, since version 1.2.5 does not work with rack >= 2.1.0"

What do you think?

@dblock
Copy link
Member

dblock commented Nov 18, 2020

Your suggestions sound good! Go for it.

Support for `rack >= 2.1.0` starts with `grape 1.3.0`
@maxkerp
Copy link
Contributor Author

maxkerp commented May 4, 2021

Sorry I totally forgot about this 馃槄 In case it's still relevant I updated it 鉁岋笍

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Thanks. Can I nitpick?

UPGRADING.md Outdated
@@ -189,6 +189,8 @@ end

### Upgrading to >= 1.3.0

You will need to upgrade to this version if you depend on `rack >= 2.1.0`
Copy link
Member

Choose a reason for hiding this comment

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

It's missing a period.

Well, I missed a period.
@maxkerp
Copy link
Contributor Author

maxkerp commented May 5, 2021

Sure, you can. Added it 鉁岋笍

@dblock dblock merged commit 613f9f1 into ruby-grape:master May 5, 2021
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

3 participants