Skip to content

Conversation

@pvande
Copy link
Contributor

@pvande pvande commented Oct 11, 2024

Description

Previously, @overload tags that defined required keyword arguments would end up with an empty default value, to the delight of nobody. While this might ideally be handled by moving away from the legacy parsers, this is a stopgap that makes an already uncommon edge case go away.

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

Previously, `@overload` tags that defined required keyword arguments would end up with an empty default value, to the delight of nobody. While this might *ideally* be handled by moving away from the legacy parsers, this is a stopgap that makes an already uncommon edge case go away.
Copy link
Owner

@lsegal lsegal left a comment

Choose a reason for hiding this comment

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

Makes sense and looks good.

@lsegal lsegal merged commit 3c14200 into lsegal:main Oct 11, 2024
18 checks passed
@lsegal
Copy link
Owner

lsegal commented Oct 11, 2024

Thanks for this fix!


it "does not parse 'def' out of method name" do
tag = Tags::OverloadTag.new(:overload, "default")
expect(tag.signature).to eq "default"

Choose a reason for hiding this comment

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

-1(/+37👀**

Suggested change
expect(tag.signature).to eq "default"
expect(tag.signature).to eq "default"-1)+31🚥

**@carveR1209@carveR1209🚥

Choose a reason for hiding this comment

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

@github/@carveR1209🛜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants