Skip to content

Return list of DBAction from Start #944

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 3 commits into from
Jul 2, 2025
Merged

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Jul 1, 2025

This PR returns adds a new return value to Start to return the list of DBActions. However, returning 3 values is too many, so in a follow-up PR I am combining the list of DBActions and backfill.Task into the same data structure. The name is pending, my ideas so far StartStep or StartOperation. If you have a better name, I am open to using them.

New data structure in a follow-up PR:

type StartOperation struct {
    Actions []DBAction
    Task *backfill.Task
}

@github-actions github-actions bot temporarily deployed to Docs Preview July 1, 2025 16:36 Inactive
@kvch kvch force-pushed the refactoring-return-actions-from-start branch from 3e741e2 to 1968e8f Compare July 1, 2025 16:37
@github-actions github-actions bot temporarily deployed to Docs Preview July 1, 2025 16:37 Inactive
@kvch kvch force-pushed the refactoring-return-actions-from-start branch from 1968e8f to 9feb18a Compare July 1, 2025 16:43
@github-actions github-actions bot temporarily deployed to Docs Preview July 1, 2025 16:43 Inactive
@kvch kvch requested a review from andrew-farries July 1, 2025 16:46
Copy link
Collaborator

@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple of comments 👍

@@ -101,15 +101,20 @@ func (m *Roll) StartDDLOperations(ctx context.Context, migration *migrations.Mig
// execute operations
job := backfill.NewJob(m.schema, versionSchemaName)
for _, op := range migration.Operations {
task, err := op.Start(ctx, m.logger, m.pgConn, newSchema)
actions, task, err := op.Start(ctx, m.logger, m.pgConn, newSchema)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename task to something more specific like backfillTask.

Co-authored-by: Andrew Farries <andyrb@gmail.com>
@github-actions github-actions bot temporarily deployed to Docs Preview July 2, 2025 12:05 Inactive
@kvch kvch enabled auto-merge (squash) July 2, 2025 12:32
@github-actions github-actions bot temporarily deployed to Docs Preview July 2, 2025 12:32 Inactive
@kvch kvch merged commit 470b5ef into main Jul 2, 2025
28 checks passed
@kvch kvch deleted the refactoring-return-actions-from-start branch July 2, 2025 12:34
Copy link

github-actions bot commented Jul 2, 2025

Merging this branch changes the coverage (1 decrease, 1 increase)

Impacted Packages Coverage Δ 🤖
github.com/xataio/pgroll/pkg/migrations 76.72% (+0.62%) 👍
github.com/xataio/pgroll/pkg/roll 79.40% (-0.12%) 👎

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/xataio/pgroll/pkg/migrations/migrations.go 94.12% (ø) 17 16 1
github.com/xataio/pgroll/pkg/migrations/op_add_column.go 89.90% (+3.36%) 99 (-5) 89 (-1) 10 (-4) 👍
github.com/xataio/pgroll/pkg/migrations/op_alter_column.go 90.27% (+0.18%) 113 (+2) 102 (+2) 11 👍
github.com/xataio/pgroll/pkg/migrations/op_change_type.go 92.86% (ø) 14 13 1
github.com/xataio/pgroll/pkg/migrations/op_create_constraint.go 85.05% (+1.24%) 107 (+2) 91 (+3) 16 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_create_index.go 67.50% (ø) 40 27 13
github.com/xataio/pgroll/pkg/migrations/op_create_table.go 85.71% (+1.74%) 126 (-5) 108 (-2) 18 (-3) 👍
github.com/xataio/pgroll/pkg/migrations/op_drop_column.go 83.33% (ø) 24 20 4
github.com/xataio/pgroll/pkg/migrations/op_drop_constraint.go 92.50% (+2.02%) 40 (-2) 37 (-1) 3 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_drop_index.go 90.91% (ø) 11 10 1
github.com/xataio/pgroll/pkg/migrations/op_drop_multicolumn_constraint.go 94.74% (+1.63%) 57 (-1) 54 3 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_drop_not_null.go 93.33% (ø) 15 14 1
github.com/xataio/pgroll/pkg/migrations/op_drop_table.go 94.12% (+4.12%) 17 (-3) 16 (-2) 1 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_raw_sql.go 88.89% (ø) 18 16 2
github.com/xataio/pgroll/pkg/migrations/op_rename_column.go 93.33% (ø) 30 28 2
github.com/xataio/pgroll/pkg/migrations/op_rename_constraint.go 100.00% (ø) 16 16 0
github.com/xataio/pgroll/pkg/migrations/op_rename_table.go 100.00% (ø) 15 15 0
github.com/xataio/pgroll/pkg/migrations/op_set_check.go 91.30% (+4.35%) 23 21 (+1) 2 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_set_comment.go 90.00% (ø) 10 9 1
github.com/xataio/pgroll/pkg/migrations/op_set_default.go 89.47% (+3.76%) 19 (-2) 17 (-1) 2 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_set_fk.go 77.42% (+2.42%) 31 (-1) 24 7 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_set_notnull.go 90.48% (+4.11%) 21 (-1) 19 2 (-1) 👍
github.com/xataio/pgroll/pkg/migrations/op_set_replica_identity.go 100.00% (ø) 17 17 0
github.com/xataio/pgroll/pkg/migrations/op_set_unique.go 77.27% (ø) 22 17 5
github.com/xataio/pgroll/pkg/roll/execute.go 78.16% (-0.20%) 174 (+3) 136 (+2) 38 (+1) 👎

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

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