Skip to content
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

fix(topic): require lowerCamel topic names instead of lower_snake #2329

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

safeer
Copy link
Contributor

@safeer safeer commented Aug 12, 2024

Breaking change, fixes #2328

Original topic name PR is #1958

@github-actions github-actions bot changed the title fix!: require lowerCamel topic names instead of lower_snake fix(topic): require lowerCamel topic names instead of lower_snake Aug 12, 2024
@ftl-robot ftl-robot mentioned this pull request Aug 12, 2024
@safeer safeer marked this pull request as ready for review August 12, 2024 19:36
@safeer safeer requested review from a team and deniseli and removed request for a team August 12, 2024 19:36
Copy link
Contributor

@deniseli deniseli left a comment

Choose a reason for hiding this comment

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

Do we need to hold off on merging this? It's any easy-enough change on the customer repo, but if they need to do some sort of emergency deploy, it'd be annoying if they suddenly needed to rename all the topics at the same time.

@wesbillman
Copy link
Member

Maybe we can just plan this with the customer repo. It might be a good time to get some of this in and we can manually update the db(s) as needed to get them running again. I'm happy to help with this and pair with us/them on getting it deployed.

@alecthomas
Copy link
Collaborator

@mistermoe has said it's okay to wipe the databases (except the secrets) table

@gak also has a breaking change he wants to get in, so let's merge these changes but not deploy until we have both in

@safeer safeer added this pull request to the merge queue Aug 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 13, 2024
@wesbillman
Copy link
Member

@mistermoe has said it's okay to wipe the databases (except the secrets) table

@gak also has a breaking change he wants to get in, so let's merge these changes but not deploy until we have both in

@alecthomas I was going to help deploy the topic names PR today, but sounds like we'll need to wait for @gak's change as well, so we can do a deploy this evening or tomorrow instead 👍

@safeer safeer added this pull request to the merge queue Aug 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 13, 2024
@safeer safeer added this pull request to the merge queue Aug 13, 2024
Merged via the queue into main with commit a5425b1 Aug 13, 2024
17 checks passed
@safeer safeer deleted the saf/topic-names-lower-camel branch August 13, 2024 16:48
matt2e added a commit that referenced this pull request Aug 14, 2024
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.

Migrate naming of topics/subscriptions to lowerCamelCase
4 participants