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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants