- Issue created by @fago
- Assigned to roderik
- 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
OK. I'm fairly sure what should roughly be done here, but the code is too abstract for me to see if it's small or large.
I'll just start by hacking the optional name argument into CustomElementGenerator::process() + the addToElement() implementations, starting with DefaultContentEntityProcessor + DefaultFieldItemListProcessor.
And then see if I have questions / whether I already see what the actual practical behavior change is and how AutoCeFieldFormatter::build() should be simplified (??) when it works...
- Merge request !48Add extra $key = '' arg to CustomElementProcessorInterface::addtoElement() +... → (Merged) created by roderik
- Issue was unassigned.
- Status changed to Needs review
8 months ago 9:28pm 6 May 2024 - 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
( OK that wasn't actually very much / didn't cause any breakage. I just needed to change/test the code, in order to see things clearly. )
Created draft change record. Created MR with question about whether we should add an extra annotation.
- 🇦🇹Austria fago Vienna
hmm, this is not so "optional" as I had thought of it.
Modifying all Processor classes can be done before updating the custom_elements module to version 3.x, without impact. Immediately after updating the module, all previously unmodified Processor classes will break.
That's not good, since it could result into severe upgrade headache. e.g. at lupus_decoupled we want to support custom_elements 2 and 3 in parallel until 3 is stable. Also that means custom code must be changed when updating, what makes it complicated. I think we should check and find a way to avoid this.
>The name should be optional, to keep BC for 2.x processors.
We really need this BC I think. - 🇦🇹Austria fago Vienna
Took a stab on how it could look like in a fully BC way. What do you think about that?
https://git.drupalcode.org/project/custom_elements/-/merge_requests/49/d...
- 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
That proposal looks good.
Because of the other potentially-breaking changes we are introducing, I was getting lost a little on whether we want to prevent breakage for sites on the 3.x upgrade. But personally I always tend to favor 'not breaking things' so I agree. (And yes, this change would break things in a 'wider' way than the others.)
I amended the Change Record. I also added a note that things still break on upgrade for Processor classes which extend custom_elements processor classes -- but that's not an official contract.
I might want to sneak a sentence into CustomElementProcessorWithKeyInterface comments, referring to the reason for needing to implement it (see Change Record). But I don't think I have push rights to your MR in the main repo, so I'll do that in another MR.
So:
RTBC for code
Needs Review for change notice. - 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
Oh -- I guess you wanted me to add your code to MR 48 because that's the one referenced here.
So I did, and I did sneak in that extra sentence. (I believe that there's plenty of room for code readers to get confused between Processors and Formatters and CE displays etc - so a minimal reference to "why?" is IMHO warranted.)
https://git.drupalcode.org/project/custom_elements/-/merge_requests/48/d...
- Status changed to Needs work
8 months ago 11:05am 7 May 2024 - Status changed to Needs review
8 months ago 2:09pm 7 May 2024 - 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
Merged 3.x (includes 📌 Separate configuration of CE field formatters and display components Fixed ), implemented feedback, build is green again.
- Status changed to Fixed
8 months ago 2:15pm 9 May 2024 - 🇦🇹Austria fago Vienna
I resolved conflicts with gitlab and merged results! Thanks!
Automatically closed - issue fixed for 2 weeks with no activity.