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.

Sign up for GitHub

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

Jump to bottom

Add annotations for presence/presence_in #182

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Add annotations for presence/presence_in #182

merged 1 commit into from
Oct 27, 2023

Conversation

bdewater
Copy link
Contributor

Type of Change

  • Add RBI for a new gem
  • Modify RBI for an existing gem
  • Other:

Changes

@bdewater bdewater requested a review from a team as a code owner October 26, 2023 20:56
@bdewater bdewater requested review from andyw8 and st0012 October 26, 2023 20:56
@andyw8
Copy link
Contributor

andyw8 commented Oct 27, 2023

Note: This adds 40 typechecking errors on Shopify/shopify, looks like most/all are solvable with T.must.

andyw8
andyw8 approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@andyw8 andyw8 left a comment

Choose a reason for hiding this comment

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

Any concerns @KaanOzkan @Morriar ?

@bdewater
Copy link
Contributor Author

40 seems to be my lucky number this week 😆

@bdewater
Copy link
Contributor Author

That made me realize presence for Active Record should also be defined since it overrides present?. Just pushed up a fix.

KaanOzkan
KaanOzkan approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@KaanOzkan KaanOzkan left a comment

Choose a reason for hiding this comment

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

lgtm

@KaanOzkan KaanOzkan merged commit 710b890 into Shopify:main Oct 27, 2023
2 checks passed
@bdewater bdewater deleted the as-presence branch October 28, 2023 04:01
@bdewater bdewater mentioned this pull request Oct 30, 2023
Merged
3 tasks
@Morriar Morriar added the rbi Change related to RBI annotations label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KaanOzkan KaanOzkan KaanOzkan approved these changes

@andyw8 andyw8 andyw8 approved these changes

@st0012 st0012 Awaiting requested review from st0012 st0012 is a code owner automatically assigned from Shopify/sorbet

Assignees
No one assigned
Labels
rbi Change related to RBI annotations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bdewater @andyw8 @KaanOzkan @Morriar