Skip to content

Conversation

@joe-yeager
Copy link
Contributor

@joe-yeager joe-yeager commented May 1, 2025

Description and Context

Add a more explicit migration warning for projects:

image

borderStyle: 'round',
};

export async function logInBox({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured it made sense to make a wrapper around this so we don't need to repeat the same options and dynamic import in multiple places.

@joe-yeager joe-yeager marked this pull request as ready for review May 1, 2025 21:48
kemmerle
kemmerle previously approved these changes May 2, 2025
Copy link
Contributor

@kemmerle kemmerle left a comment

Choose a reason for hiding this comment

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

Code looks clean, and I like the warning design! :shipit:

lib/ui/boxen.ts Outdated
export async function logInBox({
contents,
options,
fallBackToNoBox = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this variable name lol, but I'd also be fine with removing it and always falling back to no box. Or defaulting to true. We should probably be falling back to no box in the fire alarm middleware too.

Copy link
Contributor Author

@joe-yeager joe-yeager May 5, 2025

Choose a reason for hiding this comment

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

Haha, I had such a hard time naming it. I'll make it fallback by default and remove this. I think it makes sense that we would probably want to have a fallback in place at all times.

@joe-yeager joe-yeager merged commit 81c2f15 into main May 5, 2025
1 check passed
@joe-yeager joe-yeager deleted the jy/add-migration-warning branch May 5, 2025 17:21
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.

4 participants