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

[Feature] Define schema assets filter in configuration (continue) #101

Closed
wants to merge 3 commits into from

Conversation

e-vil-dev
Copy link

Continue #36 on base of 3.10.x branch + unit tests

Add support doctrine.configuration.{$name}.schema_assets_filter configuration value

example/full-config.php Outdated Show resolved Hide resolved
@e-vil-dev e-vil-dev force-pushed the schema-asserts-filter branch from f279857 to 0b6aa54 Compare February 7, 2023 21:10
@e-vil-dev e-vil-dev requested a review from Ocramius February 7, 2023 21:15
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Looking good! Mostly needs minor polishing around the example now, with focus on the reader :)


SchemaAssetsFilter::class => static function (): callable {
/**
* @param AbstractAsset|string $asset
Copy link
Member

Choose a reason for hiding this comment

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

Note: we are on PHP 8.1 or newer, so we can use union types :D

@@ -193,6 +196,20 @@

DependencyFactory::class => DependencyFactoryFactory::class,
ConfigurationLoader::class => ConfigurationLoaderFactory::class,

SchemaAssetsFilter::class => static function (): callable {
Copy link
Member

Choose a reason for hiding this comment

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

SchemaAssetsFilter is not a real class here, is it? Let's use a string, if it isn't.

Comment on lines 203 to 212
*/
return static fn (mixed $asset): bool => ! in_array(
$asset instanceof AbstractAsset ? $asset->getName() : $asset,
[
'sequence_to_generate_value',
'table_without_doctrine_mapping',
],
true,
);
Copy link
Member

Choose a reason for hiding this comment

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

Let's expand the comment (since this is reference documentation) explaining that this is mostly filtering out the two tables that are listed in the closure

@Ocramius Ocramius added the enhancement New feature or request label Feb 8, 2023
@Ocramius Ocramius added this to the 3.10.0 milestone Feb 8, 2023
@Ocramius Ocramius added the documentation Improvements or additions to documentation label Feb 8, 2023
1. Fix 'Undefined array key "schema_assets_filter"' warning by add default config value
@e-vil-dev e-vil-dev force-pushed the schema-asserts-filter branch from 0b6aa54 to dae3ee1 Compare February 8, 2023 15:17
@mrVrAlex
Copy link
Contributor

@Ocramius Any further actions on this? Or just wait release?)

@Slamdunk
Copy link
Collaborator

Merged in #107 thank you @e-vil-dev

@Slamdunk Slamdunk closed this Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants