search

LendingHome / zero_downtime_migrations

Zero downtime migrations with ActiveRecord 3+ and PostgreSQL
MIT License
561 stars 27 forks source link

Add check for drop table calls #13

Closed metalspawn closed 6 years ago

metalspawn commented 6 years ago

What has been done

I've add a check for the drop table command and provided a warning about dependant code

metalspawn commented 6 years ago

Oops, this was mean to be against our repo - might PR this a little later!

mattgmarcus commented 6 years ago

Hey Michael,

Just two comments on this PR if you do end up PR'ing to us eventually.

  1. You might want to include in your warning that this is an irreversible migration and should be in an up method as opposed to a change method.

  2. Minor nitpick, but I noticed a few typos in the comments seperate --> separate dependant --> dependent

metalspawn commented 6 years ago

Hey @mattgmarcus, thank you for taking the time to look through this anyway.

Great points, I'll definitely fold them into our own forked version for now (and don't worry about nitpicking, details are important).

The only reason why I wouldn't necessarily PR this change is because I added instructions that are specific to our deploy environment (blue/green). Would this type of change be something you'd ultimately like to include?

TBH I want to get the full gambit of troublesome migrations catered for, loosely based off: https://www.braintreepayments.com/blog/safe-operations-for-high-volume-postgresql/.