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

Add versioning to lhm-shopify methods #283

Closed
wants to merge 1 commit into from
Closed

Conversation

chitty
Copy link
Contributor

@chitty chitty commented Sep 10, 2024

Type of Change

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

Changes

Add versioning to lhm-shopify methods add_column, ddl, rename_column and remove_column

@chitty chitty requested a review from a team as a code owner September 10, 2024 20:48
Copy link
Contributor

@amomchilov amomchilov left a comment

Choose a reason for hiding this comment

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

This change is great in principle, but I'm don't think this kind of versioning is currently supported.

The only other instance I see of @version is here:

# @version >= 7.2.0.beta1
sig do
params(
error: T.any(Exception, String),
severity: T.nilable(Symbol),
context: T::Hash[Symbol, T.untyped],
source: T.nilable(String),
).void
end
def unexpected(error, severity: T.unsafe(nil), context: T.unsafe(nil), source: T.unsafe(nil)); end

In that case, it's only adding or removing a method definition its entirety, rather than trying to express one of two different versions of it.

See also: #95

@egiurleo
Copy link
Contributor

@amomchilov and I have spoken about this offline but versioning is indeed supported! I'm investigating the build failures right now.

@KaanOzkan
Copy link
Contributor

@chitty Cherry picked your changes in #286

@KaanOzkan KaanOzkan closed this Sep 30, 2024
@chitty
Copy link
Contributor Author

chitty commented Sep 30, 2024

Thank you @KaanOzkan 🙏

@thomassieczkowski
Copy link

Hey @chitty we are having these build issues that might be associated with this change. It's breaking this dependency update. Any insights on how to solve this?

@chitty
Copy link
Contributor Author

chitty commented Oct 9, 2024

@thomassieczkowski you can run bin/tapioca annotations to pull the new annotations added in #286.

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.

5 participants