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

Fix schema_assets_filter configuration docs #731

Merged

Conversation

ittmann
Copy link
Contributor

@ittmann ittmann commented Jan 25, 2023

The callable set as schemaAssetsFilter should return false for tables that you want filtered out

See for example https://github.com/doctrine/dbal/blob/3.6.x/src/Configuration.php#L168 and https://github.com/doctrine/dbal/blob/4.0.x/src/Schema/AbstractSchemaManager.php#L204

@SenseException
Copy link
Member

A description about what to expect on true and false would be insightful. Something like "If the function returns true, then..."

Copy link
Member

@driehle driehle left a comment

Choose a reason for hiding this comment

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

Yes, please add something in the writting text as well on how this filter works!

@DIReports
Copy link
Contributor

My intent with this PR was to fix the documentation error.
Documentation should probably be added to the DBAL Configuration documentation and/or Configuration class

With the latest package versions (dbal 3.5.3 and migrations 3.5.5) adding the 'migrations' table to the asset filter breaks doctrine migrations (and not adding it makes the schema-tool when run with the the "--complete" option drop the table)

@DIReports
Copy link
Contributor

Adding a table to the schema_asset_filter will not prevent an entity table from being created, only tables not owned by the DBAL from being dropped, I changed this in the first line.

Also I added some explanation on how the schema_assets_filter works and removed the 'migrations' table from the example as adding it to the asset filter currently breaks doctrine migrations.

@ittmann ittmann requested a review from driehle February 6, 2023 12:51
@ittmann
Copy link
Contributor Author

ittmann commented Feb 6, 2023

With the latest package versions (dbal 3.5.3 and migrations 3.5.5) adding the 'migrations' table to the asset filter breaks doctrine migrations (and not adding it makes the schema-tool when run with the the "--complete" option drop the table)

Opened issue doctrine/migrations#1317
Adding the migrations table to doctrine (dbal) schema assets filter breaks migrations

Fix Ignored error pattern #Parameter \doctrine#1 .* of method Doctrine\\Common\\DataFixtures\\Executor\\ORMExecutor::execute\(\)# in path
         tests/Paginator/AdapterTest.php was not matched in reported errors.
@@ -15,9 +15,5 @@ parameters:
-
message: '#Parameter \#1 .* of method DoctrineORMModule\\Options\\Configuration.* stdClass given#'
path: tests/Options/ConfigurationOptionsTest.php
-
Copy link
Member

Choose a reason for hiding this comment

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

This was removed in 7f112e9

@TomHAnderson TomHAnderson merged commit d0a100d into doctrine:5.4.x Jun 9, 2023
TomHAnderson added a commit to TomHAnderson/DoctrineORMModule that referenced this pull request Jun 9, 2023
@driehle driehle added this to the 6.0.1 milestone Oct 15, 2023
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.

5 participants