-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Mention rack 2.1.0 support in UPGRADING.md #2130
Conversation
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 :)
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 |
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? |
Well that got merged fast 😁 Always feels good to contribute to oss, even when it's a tiny bit I had the option to either upgrade to Since I want to wait with the upgrade of Since gem 'rack', '~> 2.0.0' I will remove this line from my Gemfile once I'm done upgrading to Does this answer your questions? |
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. |
Know I understand and I think this is kind of tricky. The information I thought I might find here is the incompatibility of The line I added tells me that versions <= 1.3.0 don't work with 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 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 What do you think? |
Your suggestions sound good! Go for it. |
Support for `rack >= 2.1.0` starts with `grape 1.3.0`
Sorry I totally forgot about this 😅 In case it's still relevant I updated it ✌️ |
There was a problem hiding this 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` |
There was a problem hiding this comment.
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.
Sure, you can. Added it ✌️ |
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 :)