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.

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

Detach RBI header comment from top-level node (when generating YARD docs) #1885

Merged
merged 2 commits into from
May 1, 2024

Conversation

dduugg
Copy link
Contributor

@dduugg dduugg commented Apr 27, 2024

Motivation

See Homebrew/brew#17167
When generating YARD docs that include both source and rbi files, the tapioca header overwrites the source comments for the file's top-level node.

Implementation

YARD detaches comments that have more than two newlines of separation from the source code that follows. Thus, I've added a blank line following the file header comment, and verified that the generated yard docs do not include the header.

Tests

Tests have been updated.

@dduugg dduugg requested a review from a team as a code owner April 27, 2024 23:53
@dduugg dduugg requested review from Morriar and egiurleo April 27, 2024 23:53
@dduugg dduugg changed the title Detach RBI header comment from top-level node enhancement: Detach RBI header comment from top-level node (when generating YARD docs) Apr 27, 2024
@dduugg dduugg changed the title enhancement: Detach RBI header comment from top-level node (when generating YARD docs) Detach RBI header comment from top-level node (when generating YARD docs) Apr 27, 2024
@Morriar
Copy link
Collaborator

Morriar commented Apr 29, 2024

I'm on the fence about this change as it will clash with the Layout/EmptyLines Rubocop rule without any good way of fixing it.

Could you use this solution instead? lsegal/yard#484 (comment)

@dduugg
Copy link
Contributor Author

dduugg commented Apr 29, 2024

Could you use this solution instead? lsegal/yard#484 (comment)

That's a mighty large hammer, which breaks the YARD contract for the other ~200k lines of Ruby in the repo. I would rather disable Layout/EmptyLines for the tapioca output path in the rubocop config (were I to lint the tapioca output at all, which kinda seems like a waste of compute). Let me know what you think.

@reitermarkus
Copy link

it will clash with the Layout/EmptyLines Rubocop rule

You could add rubocop:disable Layout/EmptyLines to the output just in case. Either way it makes more sense to ignore auto-generated files in RuboCop anyways.

@Morriar Morriar added the bugfix label May 1, 2024
Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

I guess you're right, those files are generated, we shouldn't care too much about Rubocop.

@Morriar Morriar merged commit 80a5bb7 into Shopify:main May 1, 2024
19 checks passed
@dduugg
Copy link
Contributor Author

dduugg commented May 1, 2024

Thanks for your reconsideration @Morriar, i really appreciate it ❤️

@dduugg dduugg deleted the detach-rbi-header branch May 1, 2024 16:38
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