-
Notifications
You must be signed in to change notification settings - Fork 78
WIP: Updating library for DBAL 4 changes. #143
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: 3.x
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| /** | ||
| * @throws RuntimeException |
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.
These two functions are too close. Add comments to each function to describe the use of each.
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.
updated
src/Schema/Table.php
Outdated
| * @return Blueprint|null | ||
| */ | ||
| public function primary($columns, $indexName = false): ?Blueprint | ||
| public function primary($columns, ?string $indexName = null, bool $isClustered = false): ?Blueprint |
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.
never use ?type; use string|null
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.
updated
src/Schema/Table.php
Outdated
| { | ||
| $columns = is_array($columns) ? $columns : [$columns]; | ||
|
|
||
| if (count($columns) === 0) { |
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.
use a bang operator (and right-space-pad it) instead of equality for code brevity
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.
updated
This PR contains updates to laravel-doctrine/migrations that allow it to work with the latest version of doctrine/migrations (3.9.5) and more importantly DBAL4.
This is tagged as Work In Progress to give some time for review of the changes and to make any necessary changes to the Upgrade.md document.
These changes have been tested against a net new Laravel 12 project, but not been attempted in an existing project being updated as of yet.
Since the Laravel Doctrine website is outdated in setup information, it might make sense to include more implementation/configuration details in the Github Readme or Wiki?