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
[WIP] Added support for doctrine annotations #5284
Conversation
@@ -173,3 +178,22 @@ private function generateFallback($name) | |||
} | |||
} | |||
} | |||
|
|||
// NEXT_MAJOR: Remove this hack together with DiExtraBundle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should add deprecations
use Symfony\Component\Finder\Finder; | ||
use Symfony\Component\Finder\SplFileInfo; | ||
|
||
final class AutoRegisterCompilerPass implements CompilerPassInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code here looks familiar 😄
} | ||
|
||
@trigger_error( | ||
'Automatic registration of annotations is deprecated since 3.14, to be removed in 4.0.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe now mention that doctrine should be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work 💪🏻
* group="myGroup", | ||
* label="myLabel", | ||
* showInDashboard=true, | ||
* translationDomain="AppBundle", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should match the namespace of the example
* showInDashboard=true, | ||
* translationDomain="AppBundle", | ||
* pagerType="", | ||
* persistFilters="", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty string? No true/false?
* ) | ||
*/ | ||
class MyAdmin extends AbstractAdmin | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add „// ...“ to make clear this class is not empty
Annotations using doctrine | ||
========================== | ||
|
||
All annotations require doctrine/annotations, it can easily be installed by composer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove „easily“
|
||
final class AutoRegisterCompilerPass implements CompilerPassInterface | ||
{ | ||
const DEFAULT_SERVICE_PREFIX = 'app.admin.'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we Need this const?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one usage for it, so yes
Any news here @core23 ? |
Could you please rebase your PR and fix merge conflicts? |
Closing this in favor of @kunicmarko20 bundles |
I am targeting this branch, because {reason}.
Changelog
To do
Subject
The DiExtraBundle has no support for symfony 4 and looks abandoned. This PR will use
doctrine/annotations
to add support for all symfony versions.