-
Notifications
You must be signed in to change notification settings - Fork 55
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
SNOW-1643309 Move version drop to app package #1610
Conversation
13af273
to
38243cd
Compare
""" | ||
Drops a version defined in an application package. If --force is provided, then no user prompts will be executed. | ||
""" | ||
is_interactive = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be confusing to have 2 similar variables. Can we do something like this instead?
if force:
interactive = False
policy = AllowAlwaysPolicy()
else:
policy = AskAlwaysPolicy() if interactive else DenyAlwaysPolicy()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was copied verbatim from https://github.com/snowflakedb/snowflake-cli/pull/1610/files#diff-7f950152d04e5fc16d6529457263e2b23df2e9f6a0023f6572d49bdb5a9a9d38L140-L147 but yeah i can change it. I'm planning a refactor of this force/interactive/policy stuff anyways
version = to_identifier(version) | ||
|
||
console.step( | ||
f"About to drop version {version} in application package {package_name}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did this message change? was there a backslash at the beginning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was a multiline string when it didn't need to be since it's only one line
Moves implementation of `snow app version drop` to `ApplicationPackageEntity.version_drop` and wires it into `snow ws version drop` too.
Pre-review checklist
Changes description
Moves implementation of
snow app version drop
toApplicationPackageEntity.version_drop
and wires it intosnow ws version drop
too.