-
Notifications
You must be signed in to change notification settings - Fork 42
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: add fs-repo-12-to-13 #162
Conversation
126afe1
to
bbdb8bd
Compare
This needs tests.
Merged config cleanup migrations into this PR, and included unit test for them + updated this issue with remaining tasks. |
This removes the need for running all legacy migrations every time `make test` or `make build` is executed. "Archived" migrations are added to ignored-migrations and that will ignore them during the build. I also removed iptb because it is used only by ignored migrations, and not having to resolve its dependency issues saves us days of useless work.
This require to update this cid https://github.com/ipfs/kubo/blob/70e604ff9999752a2773772b6dd14db048d28759/repo/fsrepo/migrations/fetcher.go#L14 once ipfs/fs-repo-migrations#162 is merged and released.
This require to update this cid https://github.com/ipfs/kubo/blob/70e604ff9999752a2773772b6dd14db048d28759/repo/fsrepo/migrations/fetcher.go#L14 once ipfs/fs-repo-migrations#162 is merged and released. Fixes #9292
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.
Can we have a list of things that we are migrating? that will make it easier for the code review to know what to check as the output.
@@ -0,0 +1,59 @@ | |||
// Package atomicfile provides the ability to write a file with an eventual |
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.
Should this file be on tools
package?
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.
Probably, this doesn't need to be done now given that would be function-less code change.
It would help making future migrations cleaner.
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.
Added some comments.
Could you remove vendor folder? I'll do a second review after that.
fs-repo-12-to-13/vendor/github.com/ipfs/fs-repo-migrations/tools/LICENSE
Show resolved
Hide resolved
Keeping user config intact when any of relevant fields was customized
This require to update this cid https://github.com/ipfs/kubo/blob/70e604ff9999752a2773772b6dd14db048d28759/repo/fsrepo/migrations/fetcher.go#L14 once ipfs/fs-repo-migrations#162 is merged and released. Fixes #9292
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.
Comments have been addresses, it looks good to me, the CI is green we have tests, so I'm merging now, I'll make a fix if we find something broken with the RC.
This require to update this cid https://github.com/ipfs/kubo/blob/70e604ff9999752a2773772b6dd14db048d28759/repo/fsrepo/migrations/fetcher.go#L14 once ipfs/fs-repo-migrations#162 is merged and released. Fixes #9292
This needs
config_test.go