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 typo #5853

Merged
merged 1 commit into from
Jun 5, 2016
Merged

Fix typo #5853

merged 1 commit into from
Jun 5, 2016

Conversation

lcobucci
Copy link
Member

@lcobucci lcobucci commented Jun 5, 2016

No description provided.

@@ -217,7 +217,7 @@ public function getRegion(array $cache)

if ( ! $this->fileLockRegionDirectory) {
throw new \LogicException(
'If you what to use a "READ_WRITE" cache an implementation of "Doctrine\ORM\Cache\ConcurrentRegion" is required, ' .
'If you want to use a "READ_WRITE" cache an implementation of "Doctrine\ORM\Cache\ConcurrentRegion" is required, ' .
'The default implementation provided by doctrine is "Doctrine\ORM\Cache\Region\FileLockRegion" if you what to use it please provide a valid directory, DefaultCacheFactory#setFileLockRegionDirectory(). '

Choose a reason for hiding this comment

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

Same typo next line (starting with The default implementation provided by doctrine is...)

@fsevestre
Copy link

It seems that you have to fix some tests

@lcobucci
Copy link
Member Author

lcobucci commented Jun 5, 2016

Already fixed 😉

@lcobucci
Copy link
Member Author

lcobucci commented Jun 5, 2016

But we have a failing test that is unrelated with this change (only for PHP 7). I will also fix it here...

@lcobucci
Copy link
Member Author

lcobucci commented Jun 5, 2016

Actually it just happens on PHP 7.0.7 (cool!)

Edit: Digging up the change log I saw that it ReflectionProperty#getValue() behavior was changed on the fix of the bug#72174.

@lcobucci
Copy link
Member Author

lcobucci commented Jun 5, 2016

The problem here is that DefaultQueryCache is trying to fetch the country property from City instead of the State.

As you can see here the $entity is always the same (City in this case) while $metadata points to an association of the $entity (State).

@Ocramius how should we tackle this problem? Maybe merging this PR and opening a new issue?

@Ocramius
Copy link
Member

Ocramius commented Jun 5, 2016

Unrelated failures are not your problem ;-)
On Jun 5, 2016 13:25, "Luís Otávio Cobucci Oblonczyk" <
notifications@github.com> wrote:

The problem here is that DefaultQueryCache is trying to fetch the country
property from City instead of the State.

As you can see here
https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Cache/DefaultQueryCache.php#L266
the $entity is always the same (City in this case) while $metadata points
to an association of the $entity (State).

@Ocramius https://github.com/Ocramius how should we tackle this
problem? Maybe merging this PR and opening a new issue?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5853 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAJakGUL8oa_sPfQH0-uvrm0_J16mAl8ks5qIrIOgaJpZM4IuUtV
.

@lcobucci
Copy link
Member Author

lcobucci commented Jun 5, 2016

lated failures are not your problem ;-)

Nice, I'll create an issue about that bug.

Edit: #5854 created

@Ocramius Ocramius added the Bug label Jun 5, 2016
@Ocramius Ocramius added this to the 2.6.0 milestone Jun 5, 2016
@Ocramius Ocramius self-assigned this Jun 5, 2016
@Ocramius Ocramius merged commit 90b7450 into doctrine:master Jun 5, 2016
@Ocramius
Copy link
Member

Ocramius commented Jun 5, 2016

Merged, will go into 2.6.0 only though, as it is not really critical :-)

Thanks, @lcobucci!

@lcobucci lcobucci deleted the patch-1 branch June 5, 2016 20:58
@lcobucci
Copy link
Member Author

lcobucci commented Jun 5, 2016

Nice, is indeed way far from being critical HAHAHA
Thanks @Ocramius 😉

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

Successfully merging this pull request may close these issues.

3 participants