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

Fix migrations and some sequel things #457

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mrbrdo
Copy link

@mrbrdo mrbrdo commented Jun 26, 2013

  • fixed migrations to be compatible with databases other than MySQL (more or less just removed backticks and used some Sequel migration methods in place where you used raw SQL)
  • fixed to use Sequel.desc and asc instead of Symbol#desc as it was deprecated

@@ -2,8 +2,6 @@

Sequel.migration do
up do
# Sequel has no database-independent way of dropping a foreign key constraint.
run "ALTER TABLE `commits` DROP FOREIGN KEY `commits_ibfk_2`"
alter_table(:commits) do
drop_column :user_id

Choose a reason for hiding this comment

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

This should be drop_foreign_key :user_id which will also drop the column. MySQL does not automatically drop foreign keys. Also in the down action it should be add_foreign_key :user_id, :users, :key => :id according to Sequel docs:

For foreign key columns, the column in the associated table that this column references. Unnecessary if this column references the primary key of the associated table, except if you are using MySQL.

Copy link
Author

Choose a reason for hiding this comment

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

I moved away from Barkeep, but you are welcome to pick this up.

Choose a reason for hiding this comment

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

What alternative did you choose in the end? The barkeep repo seems pretty much unmaintained right now.

Copy link
Author

Choose a reason for hiding this comment

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

naforo.com (disclaimer I am the author)

Choose a reason for hiding this comment

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

Looks great and a git-hook based tool should work fine with our git repos which are coupled to our Redmine which handles auth/auth. I've dropped you an invite request from my company email.

OK, enough off-topic talk :-)

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