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

docs(migrations): SNS-1977 update migrations doc to reflect new style #3584

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

dbanda
Copy link
Contributor

@dbanda dbanda commented Jan 10, 2023

update migrations doc to reflect new style

@dbanda dbanda force-pushed the dbanda/update-migration-docs branch from cdd0eba to 6153764 Compare January 10, 2023 10:40
@dbanda dbanda changed the title SNS-1977 update migrations doc to reflect new style docs(migrations): SNS-1977 update migrations doc to reflect new style Jan 10, 2023
@dbanda dbanda marked this pull request as ready for review January 10, 2023 10:42
@dbanda dbanda requested a review from a team as a code owner January 10, 2023 10:42
@dbanda dbanda force-pushed the dbanda/update-migration-docs branch from 6153764 to 103f3b8 Compare January 10, 2023 11:19
Copy link
Member

@MeredithAnya MeredithAnya left a comment

Choose a reason for hiding this comment

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

smol word suggestion but otherwise LGTM

MIGRATIONS.md Outdated
@@ -105,9 +105,19 @@ The new migration should contain a class called `Migration` which inherits from

### Adding an SQL migration (ClickhouseNodeMigration)

A ClickhouseNodeMigration is the most common type of migration and determines the SQL to be run on each ClickHouse node. You should define all four methods - `forwards_local`, `backwards_local`, `forwards_dist` and `backwards_dist` in order to provide the DDL for all ClickHouse layouts that a user may have. The operations provided in the `_local` methods will run on each local ClickHouse node and the `_dist` methods will run on each distributed ClickHouse nodes (if any).
A ClickhouseNodeMigration is the most common type of migration and determines the SQL to be run on each ClickHouse node. You should define all two methods - `forwards_ops` and `backwards_ops` in order to provide the DDL for all ClickHouse layouts that a user may have. These functions return a sequence of sql ops for the forwards and backwards (reverse) migrations.
Copy link
Member

Choose a reason for hiding this comment

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

You should define all two methods ... -> You must define two methods... (I know this changes a little bit of how it was written before but I think must is better than should here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@dbanda dbanda force-pushed the dbanda/update-migration-docs branch from 103f3b8 to 8ccf3c0 Compare January 10, 2023 23:26
@dbanda dbanda enabled auto-merge (squash) January 10, 2023 23:29
@dbanda dbanda merged commit 0e8f6fb into master Jan 10, 2023
@dbanda dbanda deleted the dbanda/update-migration-docs branch January 10, 2023 23:41
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