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

ActiveSupport String inflections #593

Merged
merged 2 commits into from
Jun 10, 2024
Merged

Conversation

hibariya
Copy link
Contributor

@hibariya hibariya commented Jun 8, 2024

No description provided.

Copy link

github-actions bot commented Jun 8, 2024

@hibariya Thanks for your contribution!

Please follow the instructions below for each change.
See also: https://github.com/ruby/gem_rbs_collection/blob/main/docs/CONTRIBUTING.md

Available commands

You can use the following commands by commenting on this PR.

  • /merge: Merge this PR if CI passes

activesupport

You changed RBS files for an existing gem.
You can merge this PR yourself because you are a reviewer of this gem.
Just comment /merge to merge this PR.

You can also request a review from other reviewers if you want.

@ksss
Copy link
Collaborator

ksss commented Jun 8, 2024

$ bundle exec rbs diff --detail --format diff --type-name String --before gems/activesupport/6.0 --after 593
- [::String public] def safe_constantize: () -> untyped
+ [::String public] def safe_constantize: () -> untyped?

- [::String public] def titleize: (?keep_id_suffix: bool keep_id_suffix) -> untyped
+ [::String public] def titleize: (?keep_id_suffix: ::boolish) -> ::String

- [::String public] def demodulize: () -> untyped
+ [::String public] def demodulize: () -> ::String

- [::String public] def deconstantize: () -> untyped
+ [::String public] def deconstantize: () -> ::String

- [::String public] def parameterize: (?locale: untyped? locale, ?preserve_case: bool preserve_case, ?separator: ::String separator) -> untyped
+ [::String public] def parameterize: (?separator: ::String, ?preserve_case: ::boolish, ?locale: ::Symbol) -> ::String

- [::String public] def tableize: () -> untyped
+ [::String public] def tableize: () -> ::String

- [::String public] def classify: () -> untyped
+ [::String public] def classify: () -> ::String

- [::String public] def humanize: (?keep_id_suffix: bool keep_id_suffix, ?capitalize: bool capitalize) -> untyped
+ [::String public] def humanize: (?capitalize: ::boolish, ?keep_id_suffix: ::boolish) -> ::String

- [::String public] def upcase_first: () -> untyped
+ [::String public] def upcase_first: () -> ::String

- [::String public] def foreign_key: (?bool separate_class_name_and_id_with_underscore) -> untyped
+ [::String public] def foreign_key: (?::boolish separate_class_name_and_id_with_underscore) -> ::String

def underscore: () -> String
def dasherize: () -> String
def pluralize: (?Integer | Symbol? count, ?Symbol locale) -> String
def singularize: (?Symbol locale) -> String
def constantize: () -> untyped
def safe_constantize: () -> untyped?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since untyped includes all types, it makes no sense to make it optional.
How about to use Module??

Copy link
Contributor Author

@hibariya hibariya Jun 9, 2024

Choose a reason for hiding this comment

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

Good point 🙏 I'll remove the ? at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated: fdc3c44
@ksss Could you take a look again? Thanks!

def underscore: () -> String
def dasherize: () -> String
def pluralize: (?Integer | Symbol? count, ?Symbol locale) -> String
def singularize: (?Symbol locale) -> String
def constantize: () -> untyped
Copy link
Collaborator

Choose a reason for hiding this comment

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

Module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ksss Thanks for taking a look Yuki san! Yeah, I thought about it as well 👍 but noticed that it actually could be anything. For example, 'RUBY_VERSION'.constantize will be a string. Also, its value should basically the same as the value of Module#const_get. That's why I left these return values alone.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sense!

Copy link
Collaborator

@ksss ksss left a comment

Choose a reason for hiding this comment

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

👌

@ksss ksss merged commit 0435724 into ruby:main Jun 10, 2024
6 checks passed
soutaro pushed a commit that referenced this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants