Always use service id as block id #302
Conversation
|
helds => holds ? |
|
LGTM |
But can you? BC way possible? |
|
BTW, I think what you did is BC, am I wrong? |
The translations ids are changing :P |
Maybe keep the old one? Or don't add |
This is a BC break, because we have to change the translation keys OR the service ids. Furthermore, anybody who uses custom blocks could have new block names. |
|
👍 for changing the service ids |
Please see my previous sentences.
Not sure to understand. Can you please elaborate?
Well, some argument of why would be great. 😉 |
IMO there is no reason for keeping the old translation ids, because they are never used after this change.
For example this bundle has custom blocks. Luckily they have matching block and service id, but we can't guarantee it for every bundle in the wordl. |
| @@ -2,24 +2,24 @@ | |||
| <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2"> | |||
| <file source-language="en" datatype="plaintext" original="file.ext"> | |||
| <body> | |||
| <trans-unit id="sonata.block.container"> | |||
| <source>sonata.block.container</source> | |||
| <trans-unit id="sonata.block.service.container"> | |||
There was a problem hiding this comment.
IMO, just removing the .service will solve all our BC break issue. Am I wrong?
There was a problem hiding this comment.
If we just remove the .service in <service id="sonata.block.service... we have a BC break.
There was a problem hiding this comment.
We can't handle this without a BC break. So which name do you prefer?
|
Could you please add a note about translation changes? |
|
Note: IMO, this PR should be merged at the very end to manage possible new block service before 4.0 release. |
|
Added a note the upgrade guide. We should merge this ASAP in the master, so possible new blocks in this bundle could use the new mechanism. |
Absolutely not because new blocks in this bundle have to be added on 3.x, not master. |
core23
force-pushed
the
feature/di-name
branch
from
265f956
to
5e2b878
Compare
Should the base branch be changed then ? |
Great feature from GitHub 🎉 We can't do this in the stable branch, because this is a BC. |
|
Nice feature 🎉 |
core23
force-pushed
the
feature/di-name
branch
from
5e2b878
to
d63f79f
Compare
|
anything todo here @core23 ? |
|
As @soullivaneuh mentioned, this should be done just before releasing the 4.0 version. Or we must care that the block name and service id is the same when creating new block inside the sonata bundles. |
|
Should we close this one in favor of #344 ? |
|
No, this is the master change that will remove the BC and changes the block id's. |
|
Oh ok, great! But I think we shouldn't merge it until #344 is merged, and 3.x is merged into master, ok? |
|
Could you please rebase your PR and fix merge conflicts? |
core23
force-pushed
the
feature/di-name
branch
from
d63f79f
to
f970eab
Compare
core23
force-pushed
the
feature/di-name
branch
from
f970eab
to
4b621a0
Compare
|
Rebased |
|
Thanks @core23 ! |
Depends on #344
Changelog
Subject
The id/name of a block is automatically set to the internal service id. This is the same mechanism like creating a admin. You can't declare the block name by yourself anymore.