Skip to content

Restore Schema Dumper behavior changed by #2019#2047

Merged
koic merged 1 commit intorsim:masterfrom
yahonda:diag_39365
Sep 11, 2020
Merged

Restore Schema Dumper behavior changed by #2019#2047
koic merged 1 commit intorsim:masterfrom
yahonda:diag_39365

Conversation

@yahonda
Copy link
Collaborator

@yahonda yahonda commented Sep 10, 2020

This pull request restores Schema Dumper behavior changed by #2019
because rails/rails#39365 does not intend to dump the default primary key configuration.

Related to rails/rails#39365
Follow up #2019

private
def column_spec_for_primary_key(column)
spec = super
spec.except!(:precision) if prepare_column_options(column) == { precision: "38", null: "false" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if the comparison with the hash object is the best way, but the dumped source has been fixed.
I have a one point. I think that the hash object instantiation can be reduced by making the hash object to be a constant.

This pull request restores Schema Dumper behavior changed by rsim#2019
because rails/rails#39365 does not intend to dump the default primary key configuration.

To avoid hash creation every time, it has been frozen and saved as a
constant.

Related to rails/rails#39365
Follow up rsim#2019
Refer http://code.jeremyevans.net/presentations/rubykaigi2019/index.html#58
@yahonda
Copy link
Collaborator Author

yahonda commented Sep 11, 2020

Thanks for the review. Updated to save the frozen hash as a constant.

Copy link
Collaborator

@koic koic left a comment

Choose a reason for hiding this comment

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

Thank you for removing weird dumped table option!

@koic koic merged commit b818d53 into rsim:master Sep 11, 2020
@yahonda yahonda deleted the diag_39365 branch February 23, 2022 14:11
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