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

Correctly determine if migration is needed. #16349

Merged
merged 1 commit into from
Aug 19, 2014
Merged

Correctly determine if migration is needed. #16349

merged 1 commit into from
Aug 19, 2014

Conversation

jmcnevin
Copy link

This method would assume that if last migration in the migrations directory matched the current schema version, that the database was up to date, but this does not account for new migrations with older timestamps that may be pending.

Our dev team has been running into problems with ActiveRecord::Migration.maintain_test_schema! not always updating our test databases when two branches containing migrations are merged. We normally run db:test:prepare when this happens, but since that task is deprecated, this method probably needs to be smarter.

@gkuchta
Copy link

gkuchta commented Jul 30, 2014

👍

@rcyrus
Copy link

rcyrus commented Jul 30, 2014

👍 This is helpful.

@rafaelfranca
Copy link
Member

Seems your last commit needs a test case.

@arthurnn
Copy link
Member

could you squash the test case and the change in the same commit?

@jmcnevin
Copy link
Author

jmcnevin commented Aug 1, 2014

OK, squashed my work down to the initial commit.

@rafaelfranca
Copy link
Member

There is an extra commit in your branch. Could you remove it?

@jmcnevin
Copy link
Author

jmcnevin commented Aug 4, 2014

Removed. @JackDanger, you may want to submit that commit in a separate PR?

@rafaelfranca
Copy link
Member

@jmcnevin his commit is already on master.

@jmcnevin
Copy link
Author

jmcnevin commented Aug 4, 2014

Ahh OK, must have been a wonky rebase on my part.

This method would assume that if last migration in the migrations
directory matched the current schema version, that the database was up
to date, but this does not account for new migrations with older
timestamps that may be pending.
rafaelfranca added a commit that referenced this pull request Aug 19, 2014
Correctly determine if migration is needed.
@rafaelfranca rafaelfranca merged commit 315de6e into rails:master Aug 19, 2014
rafaelfranca added a commit that referenced this pull request Aug 19, 2014
Correctly determine if migration is needed.
rafaelfranca added a commit that referenced this pull request Aug 19, 2014
Correctly determine if migration is needed.
rafaelfranca added a commit that referenced this pull request Aug 19, 2014
Correctly determine if migration is needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants