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

Change the column length of the version column to default to 14 #773

Merged
merged 1 commit into from
Jan 3, 2019

Conversation

jwage
Copy link
Member

@jwage jwage commented Jan 2, 2019

fixes #772

The version column by default is only 14 characters so we don't need to create a varchar(255) by default. This also causes issues with MariaDB see #514 and #538 and doctrine/DoctrineMigrationsBundle#247

@@ -43,7 +43,7 @@ class Configuration
private $migrationsColumnName = 'version';

/** @var int */
private $migrationsColumnLength = 255;
private $migrationsColumnLength = 14;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private $migrationsColumnLength = 14;
private $migrationsColumnLength = 14; // strlen((new \DateTime())->format('YmdHis'))

Or maybe initialize it with that call in the constructor?

Copy link
Member Author

Choose a reason for hiding this comment

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

What is your reasoning for wanting to do it that way?

Copy link
Member

@greg0ire greg0ire Jan 2, 2019

Choose a reason for hiding this comment

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

Magic numbers are code smells I try to avoid by always showing what they represent, be it by using a constant, or showing what computation gave that result. For instance, I would use 60 * 60 * 24 * instead of 86400.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I made the change.

@jwage jwage force-pushed the change-default-version-length branch from 4a2e19c to 67b875e Compare January 2, 2019 21:41
@jwage jwage merged commit 67c8807 into master Jan 3, 2019
@jwage jwage deleted the change-default-version-length branch January 3, 2019 01:14
@jwage jwage modified the milestones: 2.0.0-beta2, 2.0.0-rc2, 2.0.0 Jan 3, 2019
mhujer added a commit to mhujer/migrations that referenced this pull request Jan 19, 2019
@mhujer mhujer mentioned this pull request Jan 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change default migrations table version column length from 255 to 14
3 participants