Skip to content

Prepare to introduce rbs and add a configuration file #238

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

Merged
merged 4 commits into from
May 8, 2025

Conversation

ericgpks
Copy link
Contributor

@ericgpks ericgpks commented May 3, 2025

What I did in this PR

To introduce rbs to this repository, I added the necessary gems like steep and typeprof to gemspec file.
I use these two gems to get support for the RBS file's type checking and type generation.
I also added a Steepfile, which includes how to check the RBS files. The configuration of Steepfile is minimal, such as which directory to put RBS files in and not to raise an error with type checking at first.

What I didn't do in this PR

This is a PR for preparation to introduce RBS to this repository, I added only configuration files.
The actual RBS file will be added in the next PR with a new dataset.

ericgpks added 2 commits May 3, 2025 22:35
To add types, I would like to have help from typeprof to define rbs files for current library code that's why I added both steep and typeprof to gemspec.
This file indicates that type definition files are placed in the `sig` directory and the actual implementation files that I want to define types for are located in the `lib` directory.
Additionally, it is configured to disable type error checking so as not to raise much errors.
@ericgpks ericgpks changed the title [wip] introduce rbs and add a configuration file preparation to introduce rbs and add a configuration file May 6, 2025
@ericgpks ericgpks marked this pull request as ready for review May 6, 2025 06:07
Steepfile Outdated
Comment on lines 6 to 7

configure_code_diagnostics(D::Ruby.silent) # `silent` diagnostics setting
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to prevent errors from RBS file inconsistencies from blocking other feature development.

Copy link
Member

Choose a reason for hiding this comment

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

OK. Could you share the documentation for this configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks.

Based on it, how about using lenient instead of silent?

It seems that silent reports nothing even for a RBS syntax error: https://github.com/soutaro/steep/blob/master/manual/ruby-diagnostics.md#rubyannotationsyntaxerror
Is it useful for us?

It seems that lenient reports an error only for a RBS syntax error. It reports something or nothing for other cases.
It will be useful for us. For example, we may be able to check our RBS in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your advice and I agree with that.
I updated with this commit e615a4b.

Comment on lines 46 to 47
spec.add_development_dependency("steep")
spec.add_development_dependency("typeprof")
Copy link
Member

Choose a reason for hiding this comment

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

Ah, could you use Gemfile for development dependencies?

(We should move all add_development_dependencys eventually but we should work on it as a separated PR.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved added gems to Gemfile with this commit. 022ada1

@kou kou changed the title preparation to introduce rbs and add a configuration file Prepare to introduce rbs and add a configuration file May 6, 2025
since want to write all of them in Gemfile in the future
@ericgpks ericgpks requested a review from kou May 6, 2025 06:58
@kou kou merged commit a15d0b9 into red-data-tools:master May 8, 2025
9 checks passed
@kou
Copy link
Member

kou commented May 8, 2025

Let's try this.

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