Skip to content

Conversation

@KaanOzkan
Copy link
Contributor

@KaanOzkan KaanOzkan commented Jun 4, 2025

Motivation

YardDoc listener thinks they are method comments and adds it to the RBI when they are already translated to sigs.

Implementation

I'm trying to naively identify RBS comments through : and |

Tests

@KaanOzkan KaanOzkan requested a review from a team as a code owner June 4, 2025 19:22
@KaanOzkan KaanOzkan marked this pull request as draft June 4, 2025 19:23
"shareable_constant_value:",
"rubocop:",
"@requires_ancestor:",
": ",
Copy link
Contributor Author

@KaanOzkan KaanOzkan Jun 4, 2025

Choose a reason for hiding this comment

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

This is used with an include? call down below. I want to make sure I'm not ignoring more comments than necessary so added the space. But it's not perfect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think prefer a fully fledge rbs_comment? method we call from documentation_comments.

Or at least a comment explaining what is this : :

Suggested change
": ",
": ", # ignore RBS signature comments

Copy link
Member

Choose a reason for hiding this comment

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

Should | be included here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Done.

@KaanOzkan KaanOzkan marked this pull request as ready for review June 4, 2025 19:27
KaanOzkan added 2 commits June 4, 2025 17:44
YardDoc listener thinks they are method comments and adds it to the RBI
when they are already translated to sigs.
@KaanOzkan KaanOzkan requested review from Morriar and st0012 June 5, 2025 13:12
@KaanOzkan KaanOzkan merged commit b0bb8b5 into main Jun 5, 2025
17 checks passed
@KaanOzkan KaanOzkan deleted the ko/yard-doc-rbs branch June 5, 2025 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants