-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
feat: message banners table migration #5009
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
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.
LGTM!
id SERIAL PRIMARY KEY NOT NULL, | ||
enabled BOOLEAN DEFAULT true NOT NULL, | ||
message TEXT NOT NULL, | ||
variant TEXT, |
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.
Is this meant as type of message? Like warning, info, error etc?
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.
Yep, exactly! Do you think we should have a different type here?
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 was just pondering the use case, as the name was so close to a part of our domain (variants and features/toggles). We could call it message_type or banner_type? Just suggestions though
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.
That's a fair point 🤔 I think I prefer to proceed like this to ensure existing external message banner payloads don't break, but it's something I'll take into account and get back to if we identify some confusion.
https://linear.app/unleash/issue/2-1485/db-create-migration-for-a-new-internal-message-banners-table
Adds a DB migration for a new
message_banners
table.