Skip to content

Add simple RBS signatures for generated files#104

Merged
mtsmfm merged 3 commits into
mtsmfm:mainfrom
joker1007:add-rbs
Jun 19, 2024
Merged

Add simple RBS signatures for generated files#104
mtsmfm merged 3 commits into
mtsmfm:mainfrom
joker1007:add-rbs

Conversation

@joker1007

@joker1007 joker1007 commented May 21, 2024

Copy link
Copy Markdown
Contributor

This pull request add rbs signatures for generated files.

Union type and type alias are not supported currently. I handle them as untyped now.

nevertheless, These signatures are useful for type checking and code completion.

I also write Steepfile to enable type checking.
But, I met a mysterious DuplicatedDeclarationError.
Because I couldn't resolve this, some type-checking errors are left.

@mtsmfm

mtsmfm commented May 22, 2024

Copy link
Copy Markdown
Owner

Thanks.
Can you update gemspec to include the sig dir?
https://github.com/ruby/rbs/blob/3e76a4d743b31542d417be563a25d7c54c0e2a76/docs/gem.md#sig-directory

@joker1007

Copy link
Copy Markdown
Contributor Author

@mtsmfm
I added sig directory to spec.files.

@mtsmfm

mtsmfm commented May 22, 2024

Copy link
Copy Markdown
Owner

@joker1007 Can you rebase this branch onto the latest main? I'd like to see the result for generator workflow and CI failure should be fixed.

@joker1007

Copy link
Copy Markdown
Contributor Author

@mtsmfm
OK. I rebased this.

Comment thread Gemfile Outdated
gem "benchmark-ips"
gem "pry-byebug"
gem "activesupport"
gem "steep"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
gem "steep"
gem "steep", require: false

To fix this failure

https://github.com/mtsmfm/language_server-protocol-ruby/actions/runs/9206138738/job/25323641047

I know it's already retired though I'd like to not drop old rubies until we face huge difficulties just for my curiosity

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@joker1007 Can you apply this change?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@joker1007 Poke

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry. I forgot to apply the change.
I fixed it.

@mtsmfm

mtsmfm commented May 23, 2024

Copy link
Copy Markdown
Owner

Sorry for bothering you, could you rebase this branch again?

Hopefully this change fixes the generator CI for forked repos

#106

@joker1007

Copy link
Copy Markdown
Contributor Author

No problem!
I rebased this.

@mtsmfm mtsmfm merged commit 15d4388 into mtsmfm:main Jun 19, 2024
@mtsmfm

mtsmfm commented Jun 19, 2024

Copy link
Copy Markdown
Owner

Thanks!

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.

2 participants