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.

Sign up for GitHub

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

Jump to bottom

Silence graphql_definition deprecation warning before suporting GQL 2.0 #217

Merged
merged 5 commits into from
Mar 9, 2022
Merged

Silence graphql_definition deprecation warning before suporting GQL 2.0 #217

merged 5 commits into from
Mar 9, 2022

Conversation

janraasch
Copy link
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
Contributor Author

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
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.

lib/graphql_devise/schema_plugin.rb
@@ -113,7 +113,11 @@ def authenticate_option(field, trace_data)
auth_required = if trace_data[:context]
field.metadata[:authenticate]
else
field.graphql_definition.metadata[:authenticate]
if Gem::Version.new(GraphQL::VERSION) >= Gem::Version.new('1.13.1')
Copy link
Member

Choose a reason for hiding this comment

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

✅

Appraisals
@@ -82,20 +82,6 @@ appraise 'rails5.2-graphql1.12' do
gem 'rspec-rails', '< 4.0'
end

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

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
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
Member

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