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

OrmTestCase: Always use full AnnotationReader, drop legacy versions #6549

Merged

Conversation

Majkl578
Copy link
Contributor

Develop was failing locally due to class imports not being recognized. That's because SimpleAnnotationReader was in use rather than full AnnotationReader.
Also not sure why this was testing version of Common - annotations are separate package for a while now. :/
Dropped legacy setup & unused $alias parameter for Doctrine\Tests\OrmTestCase#createAnnotationDriver().

cc @guilhermeblanco

@Ocramius Ocramius self-assigned this Jul 12, 2017
@Ocramius Ocramius added this to the 3.0 milestone Jul 12, 2017
@Ocramius
Copy link
Member

LGTM 👍

@Ocramius Ocramius merged commit 884feea into doctrine:develop Jul 12, 2017
@Ocramius Ocramius changed the title [develop] OrmTestCase: Always use full AnnotationReader, drop legacy versions OrmTestCase: Always use full AnnotationReader, drop legacy versions Jul 12, 2017
@Majkl578 Majkl578 deleted the dev/OrmTestCase-fix-annotation-reader branch July 25, 2017 20:29
@greg0ire greg0ire removed this from the 3.0.0 milestone Jun 27, 2021
@carnage
Copy link
Contributor

carnage commented Jul 10, 2021

This doesn't apply cleanly to the code in 2.10.x and doesn't look like it makes sense to apply it:

    /**
     * @param mixed $alias
     */
    protected function createAnnotationDriver(array $paths = [], $alias = null): AnnotationDriver
    {
        // Register the ORM Annotations in the AnnotationRegistry
        $reader = new Annotations\SimpleAnnotationReader();

        $reader->addNamespace('Doctrine\ORM\Mapping');

        if (class_exists(Annotations\PsrCachedReader::class)) {
            $reader = new Annotations\PsrCachedReader($reader, new ArrayAdapter());
        } else {
            $reader = new Annotations\CachedReader($reader, DoctrineProvider::wrap(new ArrayAdapter()));
        }

        Annotations\AnnotationRegistry::registerFile(__DIR__ . '/../../../lib/Doctrine/ORM/Mapping/Driver/DoctrineAn
notations.php');

        return new AnnotationDriver($reader, (array) $paths);
    }

Possible to replace the top two lines with a creation of the full reader instead of the simple one; but not sure what the motivation behind that change was.

@greg0ire
Copy link
Member

greg0ire commented Jul 10, 2021

I've discussed this with @beberlei the other day, the goal is to be able to drop SimpleAnnotationReader entirely, which was useful when Doctrine was the only library using annotations, but given the popularity of this lib, it's not useful anymore.

More details about this in https://github.com/doctrine/orm/search?q=SimpleAnnotationReader&type=issues

I remember @beberlei told me that would imply importing use statements in for every annotation in test files, which should be automatable.

@greg0ire
Copy link
Member

FYI: I'm on it.

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.

4 participants