Skip to content

Conversation

@chewbakartik
Copy link
Contributor

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?

@chewbakartik chewbakartik mentioned this pull request Dec 11, 2025
}

/**
* @throws RuntimeException
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

* @return Blueprint|null
*/
public function primary($columns, $indexName = false): ?Blueprint
public function primary($columns, ?string $indexName = null, bool $isClustered = false): ?Blueprint
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

{
$columns = is_array($columns) ? $columns : [$columns];

if (count($columns) === 0) {
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

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