gem_rbs_collection icon indicating copy to clipboard operation
gem_rbs_collection copied to clipboard

Fix type signature for `ActiveRecord::Base.validate` (to better support official Ruby documentation)

Open yuzisee opened this issue 7 months ago • 1 comments

According to https://guides.rubyonrails.org/v6.0/active_record_validations.html#combining-validation-conditions

the official documentation says you can have if: [Proc.new { |c| c.market.retail? }, :desktop?], so, RBS signatures should reflect that.

yuzisee avatar May 18 '25 03:05 yuzisee

@yuzisee 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

activerecord

You changed RBS files for an existing gem. You need to get approval from the reviewers of this gem.

@hibariya, @ksss, @Little-Rubyist, @tk0miya, please review this pull request. If this change is acceptable, please make a review comment including APPROVE from here. Screen Shot 2024-03-19 at 14 13 36

After that, the PR author or the reviewers can merge this PR. Just comment /merge to merge this PR.

github-actions[bot] avatar May 18 '25 03:05 github-actions[bot]

Thank you for your mention. I sincerely apologize. It completely slipped my mind.

I'm not sure why you added test cases to ActiveRecord in spite of modifying ActiveModel. Is it better to move them to ActiveModel?

tk0miya avatar Aug 26 '25 04:08 tk0miya

LGTM for this, but I could not approve this. On my local, reviewing UI is not displayed to me.

@ksss Can you approve this?

tk0miya avatar Sep 03 '25 16:09 tk0miya

@yuzisee @tk0miya Thanks!

ksss avatar Sep 04 '25 01:09 ksss

Thanks for your review, @ksss!

@yuzisee, @ksss This PR is ready to be merged. Just comment /merge to merge this PR.

github-actions[bot] avatar Sep 04 '25 01:09 github-actions[bot]

@ksss thanks!

tk0miya avatar Sep 04 '25 02:09 tk0miya

/merge

yuzisee avatar Sep 04 '25 14:09 yuzisee