Enable @CustomIdGenerator() to reference services tagged as "doctrine.id_generator"
#1284
+356
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jan 28, 2021
nicolas-grekas
force-pushed
the
service-id-generator
branch
5 times, most recently
from
ebf5641 to
73ce3eb
Compare
nicolas-grekas
force-pushed
the
service-id-generator
branch
2 times, most recently
from
70d3f58 to
a62078c
Compare
|
You are changing this a lot. We will review after you are done. We will also need tests in the end. But feature is welcome. |
nicolas-grekas
force-pushed
the
service-id-generator
branch
from
a62078c to
1345b62
Compare
fabpot
added a commit
to symfony/symfony
that referenced
this pull request
Jan 28, 2021
… Uid generators (nicolas-grekas) This PR was merged into the 5.2 branch. Discussion ---------- [DoctrineBridge] add missing `@experimental` annotation on Uid generators | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Forgotten when we created the classes. Needed since I plan to remove all the `UuidV*` generators. They're useless and a more flexible approach based on doctrine/DoctrineBundle#1284 is coming. Commits ------- e380102 [DoctrineBridge] add missing `@experimental` annotation on Uid generators
|
I was fighting the CS checker ;) |
stof
requested changes
Jan 28, 2021
nicolas-grekas
force-pushed
the
service-id-generator
branch
4 times, most recently
from
aa46925 to
e61237e
Compare
nicolas-grekas
force-pushed
the
service-id-generator
branch
2 times, most recently
from
12a4f5a to
c8caaf5
Compare
|
PR is now tested and ready for review. |
nicolas-grekas
force-pushed
the
service-id-generator
branch
2 times, most recently
from
3f58beb to
a2f59c8
Compare
ostrolucky
requested changes
Jan 29, 2021
There was a problem hiding this comment.
So far looks like a can of worms to me without having doctrine/orm#7134
nicolas-grekas
force-pushed
the
service-id-generator
branch
2 times, most recently
from
8f92b94 to
dc74aba
Compare
|
Thanks for the review @ostrolucky. I fixed the most technical comments. I'll let you know when I'm done with the remaining ones. |
nicolas-grekas
force-pushed
the
service-id-generator
branch
3 times, most recently
from
a51ee30 to
8385e1c
Compare
|
Now with a doc page. Ready for another review. |
nicolas-grekas
force-pushed
the
service-id-generator
branch
from
8385e1c to
4f566fd
Compare
ostrolucky
approved these changes
Feb 1, 2021
nicolas-grekas
force-pushed
the
service-id-generator
branch
from
4f566fd to
f1799b2
Compare
nicolas-grekas
commented
Feb 12, 2021
| <service id="doctrine.uuid_generator" class="Symfony\Bridge\Doctrine\IdGenerator\UuidGenerator" public="false"> | ||
| <argument type="service" id="uuid.factory" on-invalid="ignore" /> | ||
| <tag name="doctrine.id_generator" /> | ||
| </service> |
There was a problem hiding this comment.
added back now that symfony/symfony#39507 is merged
alcaeus
requested changes
Feb 12, 2021
There was a problem hiding this comment.
This is a great addition that makes working with ID generators in Symfony so much easier, thank you very much @nicolas-grekas!
I've left some comments, but in general this LGTM!
nicolas-grekas
force-pushed
the
service-id-generator
branch
4 times, most recently
from
cef1b6c to
e174049
Compare
…ne.id_generator"
nicolas-grekas
force-pushed
the
service-id-generator
branch
from
e174049 to
148eb8f
Compare
alcaeus
approved these changes
Feb 12, 2021
|
Am I allowed to merge now that the two of you approved the PR? |
This was referenced Mar 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #759
This PR enables referencing services when using the
@CustomIdGenerator()annotation.It works by creating a service locator of all services tagged as
doctrine.id_generator, and checking that locator when a custom id generator is found. Usage: