Skip to content

graphql: Add types for GraphQL::ExecutionError#initialize #871

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
Merged
merged 2 commits into from
Jun 5, 2025
Merged

graphql: Add types for GraphQL::ExecutionError#initialize #871

merged 2 commits into from
Jun 5, 2025

Conversation

aruma256
Copy link
Contributor

@aruma256 aruma256 commented Jun 3, 2025 •
edited
Loading

Add type definitions for GraphQL::ExecutionError#initialize method.

The GraphQL::ExecutionError class accepts optional parameters for ast_node, options, and extensions.

References:

@aruma256 aruma256 force-pushed the graphql-executionerror-args branch from fc3ae74 to d6d494d Compare June 3, 2025 13:39
@aruma256 aruma256 marked this pull request as ready for review June 3, 2025 13:45
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 3, 2025

@aruma256 Thanks for your contribution!

Please follow the instructions below for each change.
See also: https://github.com/ruby/gem_rbs_collection/blob/main/docs/CONTRIBUTING.md

Available commands

You can use the following commands by commenting on this PR.

  • /merge: Merge this PR if CI passes

graphql

You changed RBS files for an existing gem.
This gem does not have reviewers. So you can merge this PR immediately if the CI passes.
We recommend you add yourself to the reviewers for this gem.

@aruma256
Copy link
Contributor Author

aruma256 commented Jun 4, 2025

This gem does not have reviewers. So you can merge this PR immediately if the CI passes.

This is my first PR to this repository, so I'd appreciate a review if possible. 🙏

ksss
ksss reviewed Jun 4, 2025
View reviewed changes
Copy link
Collaborator

@ksss ksss left a comment

Choose a reason for hiding this comment

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

I think it's mostly good 👍

gems/graphql/1.12/graphql.rbs Outdated
@@ -191,6 +191,7 @@ module GraphQL
class Error < StandardError
end
class ExecutionError < Error
def initialize: (String message, ?ast_node: untyped, ?options: Hash[untyped, untyped]?, ?extensions: Hash[untyped, untyped]?) -> void
Copy link
Collaborator

Choose a reason for hiding this comment

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

The options keyword seems deprecated.

RBS has both recommended and documented implications.
I don't think deprecated arguments should be written in the signature.

https://github.com/rmosolgo/graphql-ruby/blob/4ecf14faf77262fcfe4ff8231f131e6b7dadc09b/lib/graphql/execution_error.rb#L15

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ksss Thank you for the review! Removed the deprecated options parameter in ac6de66

@aruma256 aruma256 force-pushed the graphql-executionerror-args branch from dbb8355 to ac6de66 Compare June 4, 2025 10:51
@aruma256 aruma256 requested a review from ksss June 4, 2025 11:29
ksss
ksss reviewed Jun 5, 2025
View reviewed changes
Copy link
Collaborator

@ksss ksss left a comment

Choose a reason for hiding this comment

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

APPROVE

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 5, 2025

Thanks for your review, @ksss!

@aruma256, @ksss This PR is ready to be merged.
Just comment /merge to merge this PR.

@aruma256
Copy link
Contributor Author

aruma256 commented Jun 5, 2025

/merge

@github-actions github-actions bot merged commit 16443a5 into ruby:main Jun 5, 2025
7 checks passed
@aruma256 aruma256 deleted the graphql-executionerror-args branch June 5, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ksss ksss ksss left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aruma256 @ksss