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

Migration null #15

Closed
wants to merge 5 commits into from
Closed

Conversation

morganp
Copy link

@morganp morganp commented Jun 23, 2011

Using change_column_null migrations

@jordanbyron
Copy link

Hey Morgan,

Take a look at the documentation for change_column_nulll again. The method signature is:

change_column_null(table_name, column_name, null, default = nil)

And the method performs this operation to determine if the null constraint should be set:

null ? 'DROP' : 'SET'

So when you set null to :null => false it will return a truthy value and drop the null restraint which isn't what we are looking for. So what needs to be done is something more like this:

change_column_null :authorizations, :user_id, false

Additionally, I don't want to pull in these other 5 commits you've got here. There are a few reasons why that is: Keeping the log clean, those commits are valid and most of them do the same thing, except the last one which undoes what we want to do.

So as I said in your last pull request:

  • Revert to a point before you made those commits git reset --hard 86792275190b5df4a9a2da17e7e3e6d4aaca406d
  • Pull in the latest changes from the main project git pull git://github.com/rmu/flow.git master
  • Create the new migration with the correct usage of change_column_null
  • Run rake db:migrate and make sure the schema.rb file is correct and the null constraint is in place
  • Create one commit for the change and shoot over a fresh pull request

I know this might seem like a lof of work just to keep the logs clean, but in the end it's a good thing for you to know how to do. Plus in the future if and when you contribute to other open source projects I want to make sure you don't get the cold shoulder because your patches are cluttered with unnecessary code.

Thanks again Morgan!

checkout a new branch from the main project

@morganp
Copy link
Author

morganp commented Jun 23, 2011

ok, I will take a look at this, quick look at the schema.rb and I can see what it should look like after a successful migration.

I should have asked earlier but I can not run my tests at the minute because of this error:

method_missing': undefined method 'with_options'

I did a quick search on google and had a look in the main application file but can not see what is required for this, part of model/article.rb. Line 7
Thanks

@jordanbyron
Copy link

Hum, does running bundle exec "rake test" work? The with_options method is a rails thing: http://api.rubyonrails.org/classes/Object.html#method-i-with_options

@morganp
Copy link
Author

morganp commented Jun 23, 2011

I had to update my database.yml for the test username/passsowrd and create the Postgresql DB for testing but

bundle exec "rake test" 

did work. I have a better understanding of how to write tests in the normal flow now that I have had a look at unit/article_test.rb My unit test was a standalone file which just prodded a few things in the DB.

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.

None yet

3 participants