Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...
> Merges the values of another bubbleable metadata object with this one.
That makes it sound that the other object is merged into the one on which you make the call. In other words, that you do this:
$my_bubbleable_metadata->merge($other_bubbleable_metadata);
// $my_bubbleable_metadata is now updated!
That is not the case though. This is what you have to do:
$my_bubbleable_metadata = $my_bubbleable_metadata->merge($other_bubbleable_metadata);
Can anyone think of how to change the wording of the summary to make this clearer?
Comment | File | Size | Author |
---|---|---|---|
#13 | 3060225-13.patch | 667 bytes | christinlepson |
Comments
Comment #2
christinlepson CreditAttribution: christinlepson at Mindgrub Technologies commentedPatch to change "Merges the values of another bubbleable metadata object with this one." to "Returns a new bubbleable metadata object containing the merged data of a given bubbleable metadata object and the current bubbleable metadata object."
Comment #3
christinlepson CreditAttribution: christinlepson at Mindgrub Technologies commentedComment #4
borisson_This change is not allowed. The first line of a documentation docblock should only be one 80 cols wide line.
We can make this clearer though, but only if we follow the coding standards for writing documenation.
Comment #5
christinlepson CreditAttribution: christinlepson at Mindgrub Technologies commentedOk a new can be added, @borisson_ did you have any suggestions regarding the wording?
Comment #6
branimirpetkov CreditAttribution: branimirpetkov as a volunteer commentedChanged the description.
Comment #7
borisson_I don't have any suggestions no. Writing documentation is really hard and I'm afraid I don't have the required skills to help.
Comment #8
branimirpetkov CreditAttribution: branimirpetkov commentedComment #9
joachim CreditAttribution: joachim as a volunteer commented> Creates a bubbleable metadata object from the merge of this one with another.
is *nearly* short enough.
> Creates a new object from the merge of this one with another.
is there, but we sacrifice saying what kind of object is involved. Is the context sufficient for us to guess? There's the @return docs anyway that will say the class.
Comment #10
christinlepson CreditAttribution: christinlepson at Mindgrub Technologies commented@joachim I believe the context would be sufficient enough given the @return docs.
We could also do "Creates a new bubbleable metadata object by merging this one with another."
Comment #11
joachim CreditAttribution: joachim as a volunteer commented@clepson I think your suggestion is best -- let's go with that.
Comment #12
borisson_Yes I agree, that sounds like a good solution.
Comment #13
christinlepson CreditAttribution: christinlepson at Mindgrub Technologies commentedUpdated docs from
> Merges the values of another bubbleable metadata object with this one.
to
> Creates a new bubbleable metadata object by merging this one with another.
As suggested in #10.
Comment #14
christinlepson CreditAttribution: christinlepson at Mindgrub Technologies commentedComment #15
borisson_@clepson please don't rtbc your own patches, setting them to needs review so that someone else can mark them as rtbc is the way this usually works.
This way, there are at least 2 people that agree on approach and implementation before it comes to a core committer.
I'm going to leave this on RTBC because I agree with the change though :)
Comment #16
catchCommitted 708161c and pushed to 8.8.x. Thanks!