Skip to content

Conversation

@jdufresne
Copy link
Contributor

Improve data integrity by adding NOT NULL constraints to columns that
should always have a value. This helps catch application and library
bugs earlier by catching mishandling of data.

Fixes #572

@danielmorrison
Copy link
Member

I like it.

A couple things:

We test migrations in test/upgrade_generator_test.rb and should do that here too.

I'm confused why you made the test changes. I see that it fails without them but I'm not sure why on first glance. Seems like a smell, but maybe just a test fluke.

@jdufresne
Copy link
Contributor Author

We test migrations in test/upgrade_generator_test.rb and should do that here too.

Added this in the latest revision.

Improve data integrity by adding NOT NULL constraints to columns that
should _always_ have a value. This helps catch application and library
bugs earlier by catching mishandling of data.

Fixes collectiveidea#572
@jdufresne
Copy link
Contributor Author

I'm confused why you made the test changes. I see that it fails without them but I'm not sure why on first glance. Seems like a smell, but maybe just a test fluke.

Good thinking. I tracked this down to auditable_id being NOT NULL, so I've reverted that portion of the change for now.

@jdufresne
Copy link
Contributor Author

@danielmorrison Any thoughts on moving ahead on this?

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.

Add null constraints to migration file?

2 participants