Skip to content

Implement {Configuration#getSchemaAssetsFilter}#3308

Merged
morozov merged 1 commit intodoctrine:masterfrom
bezhermoso:custom-asset-filter-as-callable
Oct 6, 2018
Merged

Implement {Configuration#getSchemaAssetsFilter}#3308
morozov merged 1 commit intodoctrine:masterfrom
bezhermoso:custom-asset-filter-as-callable

Conversation

@bezhermoso
Copy link
Copy Markdown
Contributor

@bezhermoso bezhermoso commented Oct 4, 2018

Q A
Type feature
BC Break no
Fixed issues #3196

Summary

Gives ability to specify a callable to use for filtering schemas by potentially something other than regular expressions. Implements what's laid out by @morozov in his comment: #3196 (comment)

  • Feature implementation
  • New tests for the feature
  • Pass existing tests

@bezhermoso bezhermoso changed the title Implement {Configuration#getFilterSchemaAssetsCallable} Implement {Configuration#getSchemaAssetsFilter} Oct 4, 2018
Copy link
Copy Markdown
Member

@morozov morozov left a comment

Choose a reason for hiding this comment

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

@bezhermoso, the patch looks good overall. Please see a few comments.

@bezhermoso
Copy link
Copy Markdown
Contributor Author

@morozov, I think I addressed your comments. Let me know if I miss anything.

@bezhermoso bezhermoso force-pushed the custom-asset-filter-as-callable branch from ae88543 to 348e600 Compare October 6, 2018 04:14
@bezhermoso bezhermoso force-pushed the custom-asset-filter-as-callable branch from 348e600 to 5110a19 Compare October 6, 2018 04:36
Copy link
Copy Markdown
Member

@morozov morozov left a comment

Choose a reason for hiding this comment

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

Looks good! 🚢

@morozov morozov merged commit 18a8040 into doctrine:master Oct 6, 2018
@morozov
Copy link
Copy Markdown
Member

morozov commented Oct 6, 2018

Thank you, @bezhermoso.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants