search

thoughtbot / carnival

An unobtrusive, developer-friendly way to add comments
MIT License
499 stars 30 forks source link

Add commentSite attribute to Comment #143

Closed pbrisbin closed 9 years ago

pbrisbin commented 9 years ago

This is phase 1 of 2. Here, all existing comments are given an incorrect SiteId of 1. This is OK because we aren't using it for anything yet. After deployment, I will manually update all existing comments to have the correct SiteId. I can then follow up with Phase 2: actually rely on commentSite.

jferris commented 9 years ago

It seems like it wouldn't be crazy to write a migration that would add SiteId as NULL, set SiteId to a valid value, and then change it to be NOT NULL.

That seems preferable because it avoids the dummy value and allows you to test out the change locally and then replay it in production.

Why does 1 work as a valid value? Are we not using foreign key constraints?

pbrisbin commented 9 years ago

It seems like it wouldn't be crazy to write a migration that would add SiteId as NULL, set SiteId to valid value, and then change it to be NOT NULL.

I can't picture how this would work. From within a migration, I can't dynamically see what Env I'm in and therefore find the correct Site record to use.

That seems preferable because it avoids the dummy value and allows you to test out the change locally and then replay it in production.

I can still test things out locally with this approach. In fact, I have.

Why does 1 work as a valid value? Are we not using foreign key constraints?

1 is a valid value in all environments. It's the "robots-development" Site.