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

Add support for configurable migration column length. #538

Merged
merged 1 commit into from
May 12, 2018
Merged

Add support for configurable migration column length. #538

merged 1 commit into from
May 12, 2018

Conversation

BusterNeece
Copy link
Contributor

@BusterNeece BusterNeece commented Aug 17, 2017

Fixes #514

Because the Migrations Configuration object's createMigrationTable() function references private properties, it is difficult if not impossible to override on a per-project basis if necessary to specify a user-configurable string length. Adding another private property seems like the most low-friction way of resolving the issue.

@mikeSimonson
Copy link
Contributor

@SlvrEagle23 Can you add a test ?

Thanks for the PR

*
* @param int $columnLength The migration string column length.
*/
public function setMigrationsColumnLength($columnLength)
Copy link
Member

Choose a reason for hiding this comment

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

Missing type declaration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The codebase I'm PR'ing against doesn't have type declarations, so I was trying to stay consistent with that.

Copy link
Member

Choose a reason for hiding this comment

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

The codebase has already been changed to require PHP 7.1 😉

*
* @return int $migrationsColumnLength The migration string column length.
*/
public function getMigrationsColumnLength()
Copy link
Member

Choose a reason for hiding this comment

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

Missing type declaration

@lcobucci lcobucci added this to the 1.6 milestone Aug 19, 2017
@alcaeus alcaeus modified the milestones: 1.6, 1.7 Nov 7, 2017
@jwage jwage modified the milestones: 1.7, 2.0 May 5, 2018
@jwage
Copy link
Member

jwage commented May 12, 2018

@SlvrEagle23 do you want to try updating this for 2.0? Do you mind if I update it and merge?

@BusterNeece
Copy link
Contributor Author

@jwage If you want to take it on, by all means I don't mind. I don't have the same dev environment set up as I did when I submitted it, and I updated much of my own code to use MariaDB 10.2, which allows the longer fields.

@jwage jwage merged commit f518dda into doctrine:master May 12, 2018
@jwage jwage removed the Needs Tests label May 19, 2018
@jwage jwage changed the title #514 -- Add support for configurable migration column length. Add support for configurable migration column length. Jun 6, 2018
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.

5 participants