gem_rbs_collection icon indicating copy to clipboard operation
gem_rbs_collection copied to clipboard

Proposal for Managing Rails Repositories

Open ksss opened this issue 10 months ago • 5 comments

Dear Maintainers of Rails Repositories(@tk0miya, @ydah, @hibariya, @Little-Rubyist).

Currently, Rails repositories follow a method where v6 signatures are inherited, and differences are added incrementally. While this approach keeps management costs low in the short term, I believe it has several issues:

  • Prototype-based signatures are often excessive, leading to a decrease in type-checking performance.
  • As new versions are introduced, the differences from the base version become larger, reducing sustainability.
  • It becomes impossible to stop maintaining v6.

Proposal

I propose discontinuing the inheritance of symbolic links from a specific version and managing only the necessary definitions manually.

Specifically, I suggest keeping the differential definitions while not inheriting *-generated.rbs, thereby minimizing the definitions. This approach aims to maintain sustainability without compromising performance.

For minor version differences, symbolic links may still be acceptable. However, for major version differences, I believe symbolic links should not be inherited.

What do you think? I would appreciate your feedback.

ksss avatar Feb 11 '25 13:02 ksss

We have six versions for activerecord gem (6.0, 6.1, 7.0, 7.1, 7.2, and 8.0). It's too much and hard to maintain. So, +1 for changing maintenance policy.

According to the maintenance policy of Rails, 6.x becomes EOL now. But 7.0+ is still supported. How about dropping symlinks to 6.x versions? https://rubyonrails.org/maintenance

In addition to this, how about merging minor versions into one version? IMO, 7.0, 7.1, and 7.2 can be merged. Then there are only 2 or 3 versions we have to maintain at the same time.

tk0miya avatar Feb 13 '25 16:02 tk0miya

Thank you for your suggestion. I’m +1 on removing the symbolic link for the major version. As for the integration of minor versions, I’m concerned about potential exceptions. I haven’t checked if this has happened in the past, but I feel like there could be cases where unintended nullable get fixed.

Little-Rubyist avatar Feb 17 '25 01:02 Little-Rubyist

@tk0miya @Little-Rubyist Thank you for response!

Proposal

How about the following policy?

Only the two most recent versions should be updatable.

  • Released Rails version: 8.0, 7.2, 7.1, ... * We support RBS just: 8.0, 7.2
  • Released Rails version: 8.1. 8.0, 7.2, 7.1, ... * We support RBS just: 8.1, 8.0

The reason is Rails' maintenance policy and our limited ability to manage rails's RBS.

If you agree with this policy, let's move on to the next discussion.
If you disagree, please share your thoughts.

Reference Information

Differences between versions

There are slight differences between minor versions.

7.0 - 7.1
$ bundle exec rbs -I gems/activesupport/7.0 -I gems/activemodel/7.0 diff --format diff --detail --type-name ActiveRecord::Base --before gems/activerecord/7.0 --after gems/activerecord/7.1
- -
+ [::ActiveRecord::Normalization public] def normalize_attribute: (untyped name) -> void

- -
+ [::ActiveRecord::TokenFor public] def generate_token_for: (::Symbol purpose) -> ::String

- -
+ [::ActiveRecord::Normalization::ClassMethods public] def self.normalizes: (*untyped names, with: untyped, ?apply_to_nil: bool) -> void

- -
+ [::ActiveRecord::Normalization::ClassMethods public] def self.normalize_value_for: (untyped name, untyped value) -> untyped

- -
+ [::ActiveRecord::SecurePassword::ClassMethods public] def self.authenticate_by: (untyped attributes) -> instance?

- -
+ [::ActiveRecord::TokenFor::ClassMethods public] def self.generates_token_for: (::Symbol purpose, ?expires_in: ::ActiveSupport::Duration) ?{ () [self: instance] -> untyped } -> untyped

- -
+ [::ActiveRecord::TokenFor::ClassMethods public] def self.find_by_token_for: (::Symbol purpose, ::String token) -> untyped

- -
+ [::ActiveRecord::TokenFor::ClassMethods public] def self.find_by_token_for!: (::Symbol purpose, ::String token) -> untyped
7.1 - 7.2
$ bundle exec rbs -I gems/activesupport/7.0 -I gems/activemodel/7.1 diff --format diff --detail --type-name ActiveRecord::Base --before gems/activerecord/7.1 --after gems/activerecord/7.2
- [::ActiveRecord::Persistence::ClassMethods public] def self.insert: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped
+ [::ActiveRecord::Querying public] def self.insert: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped

- [::ActiveRecord::Persistence::ClassMethods public] def self.insert_all: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped
+ [::ActiveRecord::Querying public] def self.insert_all: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped

- [::ActiveRecord::Persistence::ClassMethods public] def self.insert!: (untyped attributes, ?returning: untyped?, ?record_timestamps: bool?) -> untyped
+ [::ActiveRecord::Querying public] def self.insert!: (untyped attributes, ?returning: untyped?, ?record_timestamps: bool?) -> untyped

- [::ActiveRecord::Persistence::ClassMethods public] def self.insert_all!: (untyped attributes, ?returning: untyped?, ?record_timestamps: bool?) -> untyped
+ [::ActiveRecord::Querying public] def self.insert_all!: (untyped attributes, ?returning: untyped?, ?record_timestamps: bool?) -> untyped

- [::ActiveRecord::Persistence::ClassMethods public] def self.upsert: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped
+ [::ActiveRecord::Querying public] def self.upsert: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped

- [::ActiveRecord::Persistence::ClassMethods public] def self.upsert_all: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped
+ [::ActiveRecord::Querying public] def self.upsert_all: (untyped attributes, ?unique_by: untyped?, ?returning: untyped?, ?record_timestamps: bool?) -> untyped

- -
+ [::ActiveRecord::ConnectionHandling public] def self.lease_connection: () -> ::ActiveRecord::ConnectionAdapters::AbstractAdapter

- -
+ [::ActiveRecord::Querying public] def self.with_recursive: (*untyped) -> untyped

- -
+ [::ActiveRecord::TokenFor] RelationMethods: singleton(::ActiveRecord::TokenFor::RelationMethods)
7.2 - 8.0
$ bundle exec rbs -I gems/activesupport/7.0 -I gems/activemodel/7.1 diff --format diff --detail --type-name ActiveRecord::Base --before gems/activerecord/7.2 --after gems/activerecord/8.0
# empty

DefinitelyTyped (TypeScript)

  • Only the latest versions are supported.
  • Previous versions can be used but cannot be updated.

ksss avatar Mar 03 '25 13:03 ksss

+1. Let's follow the upstream's policy.

tk0miya avatar Mar 04 '25 11:03 tk0miya

+1 - started running into the issues caused by this w/ ActiveStorage's specs.

armstnp avatar May 08 '25 16:05 armstnp