Skip to content

[Metadata] Add Php file resource class list factory#1030

Merged
loic425 merged 1 commit intoSylius:external-routing-filesfrom
loic425:php-file-resource-class-list-factory
Jun 6, 2025
Merged

[Metadata] Add Php file resource class list factory#1030
loic425 merged 1 commit intoSylius:external-routing-filesfrom
loic425:php-file-resource-class-list-factory

Conversation

@loic425
Copy link
Copy Markdown
Member

@loic425 loic425 commented Jun 6, 2025

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets part of #1029
License MIT

final class PhpFileResourceClassListFactory implements ResourceClassListFactoryInterface
{
public function __construct(
private readonly ResourceExtractorInterface $metadataExtractor,
Copy link
Copy Markdown
Member Author

@loic425 loic425 Jun 6, 2025

Choose a reason for hiding this comment

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

maybe $resourceExtractor or $resourceMetadataExtractor

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.

Since the whole point of the this class to call phpFileResourceMetataResourceExtractor, then would be cool to use in the name to understand it at glance without checking service definition.

Suggested change
private readonly ResourceExtractorInterface $metadataExtractor,
private readonly ResourceExtractorInterface $phpFileResourceMetadataExtractor,

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.

Very verbose, but ok.

Comment on lines +32 to +34
/**
* @inheritdoc
*/
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
/**
* @inheritdoc
*/

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.

removed

{
public function __construct(
private readonly ResourceExtractorInterface $metadataExtractor,
private readonly ?ResourceClassListFactoryInterface $decorated = null,
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.

Is it possible for .inner to be empty within decoration process?

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.

The way of having that nullable decorated is that we can switch the decorated and the decorator, even in userland if needed.

final class PhpFileResourceClassListFactory implements ResourceClassListFactoryInterface
{
public function __construct(
private readonly ResourceExtractorInterface $metadataExtractor,
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.

Since the whole point of the this class to call phpFileResourceMetataResourceExtractor, then would be cool to use in the name to understand it at glance without checking service definition.

Suggested change
private readonly ResourceExtractorInterface $metadataExtractor,
private readonly ResourceExtractorInterface $phpFileResourceMetadataExtractor,

@loic425 loic425 force-pushed the php-file-resource-class-list-factory branch 2 times, most recently from dafa2c3 to 1fcee5e Compare June 6, 2025 13:03
@loic425 loic425 force-pushed the php-file-resource-class-list-factory branch from 1fcee5e to 972f50a Compare June 6, 2025 13:07
-
name: Create-project with skeleton
run: |
set -x
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.

I added back this debug mode and that works... strange behaviour here.
Maybe that's a luck here.

@loic425 loic425 merged commit e6ce72d into Sylius:external-routing-files Jun 6, 2025
15 checks passed
@loic425 loic425 deleted the php-file-resource-class-list-factory branch June 6, 2025 14:15
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