Adds support for grape >= 1.2 #717

myxoh · 2018-10-23T14:32:45Z

This PR is required after: ruby-grape/grape#1803
to make sure that grape-swagger is compatible with grape after the breaking changes introduced in 1.2.0

Because of the nature of the changes in the grape gem, this version won't be compatible with prior versions of grape

coveralls · 2018-10-23T14:38:06Z

Coverage decreased (-44.004%) to 55.204% when pulling 06f86ac on myxoh:support1.2 into 95d5e35 on ruby-grape:master.

coveralls · 2018-10-23T14:38:07Z

Coverage increased (+0.006%) to 99.213% when pulling d67981a on myxoh:support1.2 into 224c5f2 on ruby-grape:master.

myxoh · 2018-10-23T14:52:13Z

Not sure how to handle the fact that this is dependant on the other code being merged. I've tested this locally manually using the branch and it was working

dblock

I think we should try to make this backward compatible. The only difference is the class name being extended, so you should be able to do this with an if defined?(Grape::API::Instance) or something like that.

You need a spec for next version of grape regarding mounting the API twice. What happens to swagger in that case?

dblock · 2018-10-23T19:16:53Z

Gemfile

@@ -6,7 +6,7 @@ ruby RUBY_VERSION

 gemspec

-gem 'grape', case version = ENV['GRAPE_VERSION'] || '~> 1.0'
+gem 'grape', case version = ENV['GRAPE_VERSION'] || '~> 1.2'


Leave this at 1.0 for now, but add another entry to .travis.yml with the branch, so for example

when 'INSTANCE' { git: '...', branch: '...' } # your own PR branch here when 'HEAD' ...

dblock · 2018-10-23T19:17:12Z

UPGRADING.md

@@ -1,5 +1,9 @@
 ## Upgrading Grape-swagger

+### Upgrading to >= 0.32.0
+
+This version is *only* compatible with grape >= 1.2.0


Add some info on why, link to the other PR, add a link.

myxoh · 2018-10-25T11:19:50Z

I've addressed the issue of making swagger compatible with previous versions of Grape.
I've also added a test that covers re-mounting if the version of grape is higher than 1.2

dblock · 2018-10-25T15:53:45Z

This looks great. We can keep this moving once the Grape PR is merged.

dblock · 2018-10-27T13:24:54Z

@myxoh Now that ruby-grape/grape#1803 got merged this can be updated to build against HEAD only

Makes swagger backwards-compatible again Adds a test that covers re-mounting with configuration Describes changes on this PR Removes custom branch from build process

dblock · 2018-10-29T11:49:16Z

Looks like a rubocop failure. Lock it at 0.59.2 or upgrade to 0.60.0 and rubocop -a ; rubocop --auto-gen-config?

myxoh · 2018-10-29T12:54:04Z

Thanks @dblock . Addressed.
By the way, I've sent you an e-mail regarding the request on the other thread. Let me know if you require anything else.

dblock · 2018-10-29T14:37:46Z

Gemfile

@@ -26,7 +26,7 @@ group :development, :test do
  gem 'rake'
  gem 'rdoc'
  gem 'rspec', '~> 3.0'
-  gem 'rubocop', '~> 0.59', require: false
+  gem 'rubocop', '~> 0.60', require: false


This is OK for now, but really should just be a specific version, because each new release (eg. 0.61) will produce new errors/warnings.

Not sure whether it is desirable to lock the gem, rather than tackle the new errors / warnings as they come.
If you still feel I should lock this to a version, I'll do that

I got this in #719.

We lock rubocop because it's otherwise a moving target. Yesterday ~> 0.59 resolved to 0.59, but today it resolved to 0.60, producing new errors. So the same code built green yesterday but fails to build today, which is not desirable.

New errors and warnings should totally be tackled, but not in the way of feature/fix work :)

LeFnord

nice work … thanks

LeFnord · 2018-10-29T15:09:58Z

CHANGELOG.md


 #### Features

-* Your contribution here.


please re-add it again, same for line 9

LeFnord · 2018-10-29T15:11:09Z

Gemfile

@@ -26,7 +26,7 @@ group :development, :test do
  gem 'rake'
  gem 'rdoc'
  gem 'rspec', '~> 3.0'
-  gem 'rubocop', '~> 0.59', require: false
+  gem 'rubocop', '0.60', require: false


please don't lock it, minor rubocop updates are much easier to handle then big ones

@dblock Could you guys agree on a decision? I'm happy either way as I see both views as valid, but I don't want to step over either of you

Lets take it to #719

LeFnord · 2018-10-29T15:11:55Z

lib/grape-swagger/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true

 module GrapeSwagger
-  VERSION = '0.31.2'
+  VERSION = '0.32.0'


revert, will be done on next release

LeFnord · 2018-10-29T15:14:25Z

thanks @myxoh … please also add a travis entry

myxoh · 2018-10-29T15:23:10Z

@LeFnord regarding adding a travis entry. I'm pressuming you mean against 1.12, however, that was not released yet, it's still on HEAD. Would you reckon we should wait until that's released and then add it to the matrix?

LeFnord · 2018-10-29T15:28:54Z

ok @myxoh …

LeFnord · 2018-10-29T15:50:50Z

again … great work @myxoh … will mörge it, after specs are green 😄

* Adds support for grape >= 1.2 Makes swagger backwards-compatible again Adds a test that covers re-mounting with configuration Describes changes on this PR Removes custom branch from build process * Upgrades Rubocop and autofixes indentation * Locks rubocop version to 0.60 * Rollbacks version bump * Removes HEAD from allowed failure

myxoh mentioned this pull request Oct 23, 2018

Remountable APIs, allows to re-mounting all APIs ruby-grape/grape#1803

Merged

dblock requested changes Oct 23, 2018

View reviewed changes

dblock approved these changes Oct 25, 2018

View reviewed changes

Adds support for grape >= 1.2

41dc9c6

Makes swagger backwards-compatible again Adds a test that covers re-mounting with configuration Describes changes on this PR Removes custom branch from build process

myxoh force-pushed the support1.2 branch from 6814676 to 41dc9c6 Compare October 29, 2018 11:26

Upgrades Rubocop and autofixes indentation

8fcb0ea

myxoh force-pushed the support1.2 branch from 929defd to 8fcb0ea Compare October 29, 2018 12:48

dblock reviewed Oct 29, 2018

View reviewed changes

Locks rubocop version to 0.60

c3de530

LeFnord requested changes Oct 29, 2018

View reviewed changes

dblock mentioned this pull request Oct 29, 2018

Upgraded and locked Rubocop at 0.60.0. #719

Merged

Merge branch 'master' into support1.2

61d7edd

LeFnord mentioned this pull request Oct 29, 2018

Fix: corrected termsOfService in additional info #720

Merged

myxoh added 3 commits October 29, 2018 15:37

Rollbacks version bump

3197cee

Merge remote-tracking branch 'myxoh/support1.2' into support1.2

0329a7a

Removes HEAD from allowed failure

d67981a

LeFnord approved these changes Oct 29, 2018

View reviewed changes

LeFnord merged commit 2457215 into ruby-grape:master Oct 29, 2018

Adds support for grape >= 1.2 #717

Adds support for grape >= 1.2 #717

Uh oh!

Conversation

myxoh commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Oct 23, 2018

Uh oh!

coveralls commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

myxoh commented Oct 23, 2018

Uh oh!

dblock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myxoh commented Oct 25, 2018

Uh oh!

dblock commented Oct 25, 2018

Uh oh!

dblock commented Oct 27, 2018

Uh oh!

dblock commented Oct 29, 2018

Uh oh!

myxoh commented Oct 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeFnord left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeFnord commented Oct 29, 2018

Uh oh!

myxoh commented Oct 29, 2018

Uh oh!

LeFnord commented Oct 29, 2018

Uh oh!

LeFnord commented Oct 29, 2018

Uh oh!

Uh oh!

myxoh commented Oct 23, 2018 •

edited

Loading

coveralls commented Oct 23, 2018 •

edited

Loading