-
Notifications
You must be signed in to change notification settings - Fork 524
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
Include migration builds in image builds #716
Conversation
47c3996
to
e28edc3
Compare
Removed Ben's commit |
fddc726
to
5aa667e
Compare
Updated this to remove the This change rolls the migration logic into |
cargo make
target that builds migrationsThere 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.
Looks good! There's a couple suggestions and questions inline. Approach seems sound to me otherwise!
efac087
to
9eb641b
Compare
Reworked this quite a bit.
|
9eb641b
to
7f117d5
Compare
Addressed @bcressey 's comments |
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.
Would you please show the location/contents of the new artifact in the description?
for version_path in %{_builddir}/workspaces/api/migration/migrations/*; do | ||
for migration_path in "${version_path}"/*; do |
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.
nit: we should be consistent about quoting. (I don't like quoting paths containing globs, but if shellcheck complains, it's fine.)
This change includes migration builds with image builds. It will build all of the migrations, lz4 compress them, and tar them up as an output of the build.
7f117d5
to
9559bfc
Compare
Added contents of migrations artifact to description Addressed @tjkirch 's comments |
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.
⛵️
Issue #, if available:
Related to #606
Description of changes:
This change includes migration builds with image builds. It
will build all of the migrations, strip them, lz4 compress them, and
tar them up as an output of the build.
Testing done:
cargo make
outputs expected artifacts and migrationsBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.