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

Trigger the standard autoloader as the last resort #271

Merged
merged 5 commits into from
Apr 2, 2020

Conversation

sanmai
Copy link
Contributor

@sanmai sanmai commented Jun 28, 2019

As v2 isn't here yet, and looking at #232 it is unclear if it will be coming anytime soon, and as we have to put up with annoying deprecated functions, here I propose to fall back to the standard autoloader if nothing else worked, and if there's no custom loader defined.

Fixes #270

I intentionally did not add any tests as this is a PoC. Please tell me if this makes any sence, and then I'll make a try with tests.

@stof
Copy link
Member

stof commented Aug 20, 2019

shouldn't it check for both $loaders and $autoloadNamespaces if it want to check that nothing was registered explicitly ?

@sanmai
Copy link
Contributor Author

sanmai commented Aug 20, 2019

@stof fair, please see if it is any better now

@stof
Copy link
Member

stof commented Aug 20, 2019

👍 for this, as it means that not registering anything anymore will produce the behavior available in doctrine/annotations 2 (but without forcing anything on existing projects until they remove their explicit registration)

@stof
Copy link
Member

stof commented Aug 20, 2019

there is actually still 1 case changing the behavior for existing projects: when only registerFile was used.

@sanmai
Copy link
Contributor Author

sanmai commented Aug 21, 2019

Well, we can toggle a flag when registerFile is used, then proceeding as we do for other methods, but shall we?

@sanmai
Copy link
Contributor Author

sanmai commented Sep 21, 2019

Isn't you pre-load a class when you use AnnotationRegistry::registerFile? E.g. if you have a class loaded this way, then autoloading won't even trigger. Is there something else I miss here?

@alcaeus alcaeus changed the base branch from 1.6 to master April 1, 2020 11:25
@stof
Copy link
Member

stof commented Apr 1, 2020

Well, if you used registerFile, we might still try to discover a different annotation.

@alcaeus
Copy link
Member

alcaeus commented Apr 1, 2020

@sanmai I've rebased this against the new master after making some changes to our branches. Do you still want to work on this patch? If we can get registerFile behaviour sorted out properly, we can include this in the 1.9 release.

@sanmai
Copy link
Contributor Author

sanmai commented Apr 2, 2020

Yes, I will at least try to remember what's going on here.

@sanmai
Copy link
Contributor Author

sanmai commented Apr 2, 2020

@alcaeus please let me know if this is a passable workaround

sanmai and others added 4 commits April 2, 2020 07:59
As v2 isn't here yet, and looking at doctrine#232 it is unclear if will, and as we have to put up with annoying deprecated functions, here I propose to fall back to the standard autoloader if nothing else worked, and if there's no custom loader defined.

Fixes doctrine#270
@alcaeus
Copy link
Member

alcaeus commented Apr 2, 2020

@sanmai this looks good. I've added a couple of tests and updated the deprecation messages. Pending an approval by @stof this is ready for 1.9. Thank you very much for this improvement!

@alcaeus alcaeus added this to the 1.9.0 milestone Apr 2, 2020
alcaeus
alcaeus previously approved these changes Apr 2, 2020
Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

self::$registerFileUsed must be reset to false when resetting the registry to me.

@sanmai
Copy link
Contributor Author

sanmai commented Apr 2, 2020

@stof done

@alcaeus alcaeus merged commit 233e366 into doctrine:master Apr 2, 2020
@alcaeus alcaeus self-assigned this Apr 2, 2020
@alcaeus
Copy link
Member

alcaeus commented Apr 2, 2020

Thanks @sanmai!

@sanmai
Copy link
Contributor Author

sanmai commented Apr 2, 2020

Thank you @stof and @alcaeus!

@sanmai sanmai deleted the patch-2 branch April 2, 2020 08:16
@sanmai
Copy link
Contributor Author

sanmai commented Apr 2, 2020

It seems like this PR didn't get into 1.9, even though mentioned in the changelog.

Please confirm.

@alcaeus alcaeus modified the milestones: 1.9.0, 1.10.0 Apr 2, 2020
@sanmai
Copy link
Contributor Author

sanmai commented Apr 2, 2020

All clear. Thank you.

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.

AnnotationRegistry is deprecated but has no alternative in v1.6.1
3 participants