Skip to content

Conversation

@josiasds
Copy link
Member

The last parameter of change_column_null only affects existing records, thus the new default value still needed to be set by change_column_default.

Fixes #110.

The last parameter of change_column_null only affects existing records, thus the default still needs to be set in another migration.
@josiasds
Copy link
Member Author

@justin808 Ready for review.

Copy link
Member

Choose a reason for hiding this comment

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

Why 2 migrations?

Does change_column_null handle migrating old records? We might need a couple lines of SQL to update the existing records. Then again, there probably aren't any, or else the UI would have crashed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, yes, that last parameter (empty string) of change_column_null replaces null values of existing records, so we don't need SQL statements here.

But it still doesn't set the column's default for new records, thus we do need a second migration to change_column_default.

justin808 added a commit that referenced this pull request Oct 15, 2015
Set comment's author and text as not null and defaults as empty string
@justin808 justin808 merged commit ac8c4a8 into master Oct 15, 2015
@justin808
Copy link
Member

Thanks @josiasds

@robwise robwise deleted the josias-not-null-database-fields branch October 22, 2015 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants