Skip to content

Silence graphql_definition deprecation warning before suporting GQL 2.0#217

Merged
mcelicalderon merged 5 commits intographql-devise:releasefrom
janraasch:silence-deprecation-graphql-definition
Mar 9, 2022
Merged

Silence graphql_definition deprecation warning before suporting GQL 2.0#217
mcelicalderon merged 5 commits intographql-devise:releasefrom
janraasch:silence-deprecation-graphql-definition

Conversation

@janraasch
Copy link
Copy Markdown
Contributor

@janraasch janraasch commented Mar 9, 2022 •
edited
Loading

goal Silence graphql_definition deprecation warning for a v0.18.x release.

reasoning Nothing users of this gem can do about it before a v1.0 release that would include support for GraphQL 2.0.

context

mcelicalderon and others added 5 commits March 8, 2022 11:10
@mcelicalderon
Silence graphql_definition deprecation warning before suporting GQL 2.0
cc922c1
@janraasch
Only silence deprecation warning if graphql >= 1.13.1
27438aa 
The »silence_deprecation_warning»-param was introduced with v1.13.1

See rmosolgo/graphql-ruby@3d1c24a
@janraasch
Add support for graphql 1.13
f610d93 
This is a cherry-picked version of 5642ca9 excluding the rails-7 related changes.
@janraasch
Temp skip ruby 2.5 rails 6 GQL 1.13
4ced782 
Cherry-picked 502c555
@janraasch
Skip ruby 2.2 rails 6 GQL 1.13
b138fd4 
GQL 1.13 reuires ruby 2.4
@janraasch
Copy link
Copy Markdown
Contributor Author

janraasch commented Mar 9, 2022

Hi @mcelicalderon, I gave it a go over here following your instructions and cherry-picking what I could use from master.

⚠️ beware I have no idea why these two ruby 2.5 tests fail for GQL 1.13, see e.g. https://app.circleci.com/pipelines/github/graphql-devise/graphql_devise/148/workflows/87788dbb-b9ac-4cfd-891a-3ed6bde3feed/jobs/9211. The code is here: https://github.com/rmosolgo/graphql-ruby/blob/1.13.x/lib%2Fgraphql%2Fschema%2Ffield.rb#L282.

ℹ️ I saw that you skipped those tests on master, so I cherry-picked that commit 😇.

mcelicalderon
mcelicalderon reviewed Mar 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@mcelicalderon mcelicalderon left a comment

Choose a reason for hiding this comment

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

Thank you, @janraasch, this looks great!

beware I have no idea why these two ruby 2.5 tests fail for GQL 1.13

I was able to replicate that locally, but still couldn't get to the bottom of this, happens when the graphql gem is required, so might just be test related. Anyway, not a concer of this MR.

Comment thread lib/graphql_devise/schema_plugin.rb
field.metadata[:authenticate]
else
field.graphql_definition.metadata[:authenticate]
if Gem::Version.new(GraphQL::VERSION) >= Gem::Version.new('1.13.1')
Copy link
Copy Markdown
Member

@mcelicalderon mcelicalderon Mar 9, 2022

Choose a reason for hiding this comment

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

✅

Comment thread Appraisals
gem 'rspec-rails', '< 4.0'
end

appraise 'rails6.0-graphql1.8' do
Copy link
Copy Markdown
Member

@mcelicalderon mcelicalderon Mar 9, 2022

Choose a reason for hiding this comment

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

🙌

mcelicalderon
mcelicalderon approved these changes Mar 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@mcelicalderon mcelicalderon left a comment

Choose a reason for hiding this comment

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

Thanks, I'll push a new release with this changes asap

@mcelicalderon mcelicalderon merged commit 83612ce into graphql-devise:release Mar 9, 2022
@mcelicalderon mcelicalderon added the enhancement New feature or request label Mar 9, 2022
@mcelicalderon
Copy link
Copy Markdown
Member

mcelicalderon commented Mar 9, 2022

v0.18.2 released! Thank you for the contribution, @janraasch!

@mcelicalderon mcelicalderon mentioned this pull request Mar 9, 2022
Closed
@janraasch janraasch deleted the silence-deprecation-graphql-definition branch March 10, 2022 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcelicalderon mcelicalderon mcelicalderon approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@janraasch @mcelicalderon