Skip to content

expose all attempted execution migrations - #1194

Draft
goetas wants to merge 1 commit into
save-skipped-errored-migrationsfrom
expose-other-migration-types
Draft

expose all attempted execution migrations#1194
goetas wants to merge 1 commit into
save-skipped-errored-migrationsfrom
expose-other-migration-types

Conversation

@goetas

@goetas goetas commented Aug 22, 2021

Copy link
Copy Markdown
Member
Q A
Type feature
BC Break no
Fixed issues -

* Returns all the for which there was an execution attempt
* Includes skipped, errored executed migrations.
*/
public function getAllExecutedMigrations(): ExecutedMigrationsList;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what would be here the best way to add such functionality to this interface in a backward compatible way?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Usually something like

class Foo implements MetadataStorage, AllExecutedMigrationsInterfaceThingy

but I assume you're looking for something different?

Version $version,
?DateTimeImmutable $executedAt = null,
?float $executionTime = null,
?string $reason

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you decide not to use

Suggested change
?string $reason
?string $reason = null

?

public function getExecutedMigrations(): ExecutedMigrationsList;

/**
* Returns all the for which there was an execution attempt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* Returns all the for which there was an execution attempt
* Returns all the migrations for which there was an execution attempt

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