-
Notifications
You must be signed in to change notification settings - Fork 21
Redmine support #73
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
base: master
Are you sure you want to change the base?
Redmine support #73
Conversation
|
when Time | ||
"'#{value.strftime("%m-%d-%Y %H:%M")}'" | ||
when Date | ||
"'#{value.strftime("%m-%d-%Y")}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the BINDDATE
craziness is there for a good reason. In one of my previous PRs, I learned why this is here and explained it in the comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain it here, why binddate is better solution?
end | ||
|
||
def rename_table(name, new_name) | ||
fail ActiveRecordError, 'Firebird does not support renaming tables.' | ||
end | ||
|
||
def drop_table(name, options = {}) # :nodoc: | ||
super |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the table has a sequence, you can't drop it. You need to drop the sequence first.
Unless I'm missing something, the super
should come after the sequence is dropped.
@@ -144,13 +168,19 @@ def remove_column(table_name, column_name, type = nil, options = {}) | |||
def change_column(table_name, column_name, type, options = {}) | |||
type_sql = type_to_sql(type, *options.values_at(:limit, :precision, :scale)) | |||
|
|||
execute(squish_sql(<<-end_sql)) | |||
if type == :text | |||
remove_column table_name, column_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change column is supposed to be a "safe" operation, meaning it won't delete your data. I would say dropping a column without warning would be very unsafe.
unless options[:sequence] == false | ||
sequence_name = options[:sequence] || default_sequence_name(name) | ||
drop_sequence(sequence_name) if sequence_exists?(sequence_name) | ||
end | ||
|
||
super | ||
commit_db_transaction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for all of the commit_db_transaction
invocations? execute
should commit.
Please don't forget to run the Active Record test suite against this PR. There will be plenty of failures, but it should indicate regressions. |
Added redmine support and datetime fixes