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 error message in identifiers extractor #2713

Merged
merged 1 commit into from
Apr 19, 2019
Merged

Conversation

Nek-
Copy link
Contributor

@Nek- Nek- commented Apr 7, 2019

It was probably a bad copy/paste

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets None
License MIT
Doc PR N/A

@Nek- Nek- changed the title Fix error message in identifierextractor Fix error message in identifiers extractor Apr 7, 2019
@soyuka
Copy link
Member

soyuka commented Apr 8, 2019

tests need to be updated also

@@ -42,7 +42,7 @@ public function __construct(PropertyNameCollectionFactoryInterface $propertyName
$this->resourceClassResolver = $resourceClassResolver;

if (null === $this->resourceClassResolver) {
@trigger_error(sprintf('Not injecting %s in the CachedIdentifiersExtractor might introduce cache issues with object identifiers.', ResourceClassResolverInterface::class), E_USER_DEPRECATED);
@trigger_error(sprintf('Not injecting %s in the IdentifiersExtractor might introduce cache issues with object identifiers.', ResourceClassResolverInterface::class), E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be changed to use __CLASS__ instead? And remove "the".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it a must have?

@Nek-
Copy link
Contributor Author

Nek- commented Apr 15, 2019

CI didn't start or...?

It was probably a bad copy/paste
@soyuka soyuka merged commit 0665544 into api-platform:2.4 Apr 19, 2019
@soyuka
Copy link
Member

soyuka commented Apr 19, 2019

Thanks @Nek-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants