Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Provide support for top_const_ref nodes #2

Closed
wants to merge 4 commits into from
Closed

Provide support for top_const_ref nodes #2

wants to merge 4 commits into from

Conversation

KaanOzkan
Copy link

@KaanOzkan KaanOzkan commented Nov 13, 2020

Came across these nodes when using sigs that contain ::Klass syntax to represent user defined types.

Came across these nodes when using sigs that contain ::Klass syntax to
represent user defined types.
Copy link
Contributor

@dduugg dduugg left a comment

Choose a reason for hiding this comment

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

Thanks! Could I trouble you to add tests for each node type?

# A top-level constant reference, such as ::Klass
# It contains a child node of type :const
convert(children.first)
when :const
Copy link
Contributor

Choose a reason for hiding this comment

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

let's add this to when :const_path_ref above

@KaanOzkan
Copy link
Author

Thanks! Could I trouble you to add tests for each node type?

Sure, were you thinking of extending sig_handler.rb.txt and sig_handler_spec.rb or create a new sig_to_yard_spec and sig_to_yard.rb.txt and make it more of a unit test for the convert method @dduugg ?

@dduugg
Copy link
Contributor

dduugg commented Nov 16, 2020

@KaanOzkan I'd recommend just extending the sig_handler* files. (I'm open to unit tests as well, if you feel inspired.)

@@ -56,7 +56,7 @@ def self.convert_type(node)
else
[node.source]
end
when :const_path_ref
when :const_path_ref, :const
[node.source]
when :hash, :list
Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure what a list node is and did not test it.

@dduugg
Copy link
Contributor

dduugg commented Nov 19, 2020

Looks good! However, I just realized this is under the stripe account, not dduugg, the account that publishes the gem. Could I trouble you to re-open your PRs under https://github.com/dduugg/yard-sorbet ?

In the meantime, I'll see if I can get someone at Stripe to hide this repo, add a disclaimer, or take ownership of the gem.

@KaanOzkan KaanOzkan closed this Nov 19, 2020
@KaanOzkan KaanOzkan deleted the ko/support-top_const_ref branch November 19, 2020 16:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants