Skip to content

feat: Migrations polish #1455

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

Merged
merged 4 commits into from
Apr 17, 2025
Merged

feat: Migrations polish #1455

merged 4 commits into from
Apr 17, 2025

Conversation

joe-yeager
Copy link
Contributor

Description and Context

Does the following:

  • Dedupes components in the list of components will or will not be migrated
  • When prompting for project name, validate the name in the prompt and don't fail the process if the project already exists
  • Adds a message letting the user know projects migrations are one way
  • Cleans up some debug logging

@@ -179,21 +179,30 @@ async function selectAppToMigrate(
if (migratableComponents.length !== 0) {
logger.log(
lib.migrate.componentsToBeMigrated(
`\n - ${migratableComponents.join('\n - ')}`
`\n - ${[...new Set(migratableComponents)].join('\n - ')}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These need to be deduped before we log them

Copy link
Contributor

Choose a reason for hiding this comment

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

Do dupes exist when the user has multiple instances of a given component? Like multiple cards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, they can have multiple cards, webhooks, etc.

Comment on lines +201 to +203
const promptMessage = projectConfig?.projectConfig
? `${lib.migrate.projectMigrationWarning} ${lib.migrate.prompt.proceed}`
: lib.migrate.prompt.proceed;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the migration is for a project, add the project warning prefix

Comment on lines +257 to +267
const { projectExists } = await ensureProjectExists(
derivedAccountId,
input,
{ allowCreate: false, noLogs: true }
);

if (projectExists) {
return lib.migrate.errors.project.alreadyExists(input);
}

return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fixing an issue where if the project name was already taken, we just killed the process

Copy link
Contributor

@brandenrodgers brandenrodgers left a comment

Choose a reason for hiding this comment

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

lgtm!

@joe-yeager joe-yeager merged commit 567e713 into main Apr 17, 2025
1 check passed
@joe-yeager joe-yeager deleted the jy/migrations-polish branch April 17, 2025 18:32
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.

2 participants