Skip to content

Conversation

@mushyy
Copy link
Contributor

@mushyy mushyy commented Sep 18, 2019

Sets migration command as nil rather than '' because '' still triggers a migration if set in ey.yml rather than overriding.

Testing Done before Fix

  • Deployed normally without --no-migrate or --migrate - Previous deploy command was used with migration: "rake db:seed --trace"..
  • Deployed with --migrate command - command was used by the deploy with migration: "rake db:migrate --trace".
  • Deploy with --no-migrate option - migration was still triggerd output as with migration: ''
  • Deployed with no migrate command by UI and then by CLI with no options. - CLI respected previous option which in this case was without migration

Testing Done After Fix

  • Deployed normally without overriding --no-migrate or --migrate - Previous deploy command was used with migration: "rake db:seed --trace"..
  • Deployed with --no-migrate - no migration command was sent out put was without migration
  • Deployed with no migrate command by UI and then by CLI with no options. - CLI respected previous option which in this case was without migration
  • Deployed with --migrate command - command was used by the deploy with migration: "rake db:migrate --trace".

@mushyy mushyy requested a review from ess September 18, 2019 18:21
@danmoore2205
Copy link

any chance of this getting reviewed and merged?

@ess
Copy link
Contributor

ess commented Apr 14, 2020

@danmoore2205 I have it on good authority that @mushyy will be providing the requisite information around this PR tomorrow that will allow me to merge, so it will hopefully be released by the end of the week. Apologies for the delay, both in getting the fix merged and in getting back to you.

@danmoore2205
Copy link

Awesome. Thanks to both of you!

Copy link
Contributor

@ess ess left a comment

Choose a reason for hiding this comment

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

LGTM

@ess ess merged commit 168ffaf into master Apr 16, 2020
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.

4 participants