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

feat: db migration #1458

Merged
merged 4 commits into from
Apr 12, 2022
Merged

feat: db migration #1458

merged 4 commits into from
Apr 12, 2022

Conversation

mindlesscloud
Copy link
Contributor

@mindlesscloud mindlesscloud commented Apr 7, 2022

Summary

It provides a tool to keep model definitions and the database schema in sync. All the changes in the schema will be written in the database

Key Points

  • This is a breaking change
  • New or existing documentation is updated

Description

Describe what this PR does, and aims to solve in a few sentences.

Does this close any open issues?

#1057 #1414 #1304

Current Behavior

Describe the current behaviour of this issue, if relevant.

New Behavior

Describe the new behaviour updated in this issue, if relevant.

Screenshots

Include any relevant screenshots here.

Other Information

Any other information that is important to this PR.

@mindlesscloud mindlesscloud force-pushed the db-migration branch 2 times, most recently from c6a0a13 to 58f8750 Compare April 11, 2022 08:11
Copy link
Contributor

@klesh klesh left a comment

Choose a reason for hiding this comment

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

LGTM

"gorm.io/gorm"
)

type Script interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Script is too general, how about MigrationScript

type Script interface {
Up(ctx context.Context, db *gorm.DB) error
Version() uint64
Owner() string
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we agree that owner is not needed.

m.db = db
}

func (m *migrator) register(scripts ...Script) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should pass one more field like comment to declare which plugin they belong to.
and save it into database, to help with debugging

@klesh klesh merged commit 3d239f0 into main Apr 12, 2022
@klesh klesh deleted the db-migration branch April 12, 2022 01:28
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