-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Making mass upgrades easier in airbyte ci #38333
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
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.
I can add a version bump (meta) and merge this, let me try this locally, and see if we're ok with --bump
defaulting to None
in some commands, but to patch
in up_to_date
. I think it's fine, these are just tools for us anyway.
@@ -142,7 +153,7 @@ def __init__( | |||
|
|||
async def get_repo_dir(self) -> Directory: | |||
if not self.repo_dir: | |||
self.repo_dir = await self.context.get_connector_dir() | |||
self.repo_dir = await self.context.get_repo_dir() |
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.
Uh, I wonder how that would never get caught in prod / tests
@@ -42,18 +42,23 @@ | |||
default=False, | |||
help="Create a pull request.", | |||
) | |||
|
|||
@click.option( |
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.
Should we make this consistent with migrate_to_poertry
and migrate_to_base_image
so that they all have --bump
?
I've merged these in, but in a separate PR 🙃 |
No description provided.