Follow-up to #2416409: Delete dependent config entities that don't implement onDependencyRemoval() when a config entity is deleted
+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.php
@@ -176,7 +172,11 @@ public function calculateDependencies();
* @return bool
* TRUE if the entity has changed, FALSE if not.
*
+ * @return bool
+ * TRUE if the entity has been changed as a result, FALSE if not.
+ *
It's good to add comments, but is the return value double documented?
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff.txt | 732 bytes | snehi |
| #7 | dobule-documentation-2657734-7.patch | 812 bytes | snehi |
| #6 | interdiff-2657734-6-2.txt | 636 bytes | miteshmap |
| #6 | dobule-documentation-2657734-6.patch | 674 bytes | miteshmap |
| #2 | 2657734-double-documentation-2.patch | 718 bytes | floydm |
Comments
Comment #2
floydm commentedPatch attached.
Comment #3
lokapujyaSeems like the one you removed is the newer comment. Only thing is that I wonder if it's better to keep the newer one?
Comment #4
floydm commentedBeats me. The newer one made less sense to me. "as a result" of what?
Comment #5
lokapujyaSo, I commented in the issue that added this to try to recruit some advice. But I think its as a result of changing affected entities in this method. I have to think about the exact wording. So, "as a result of this method", but the newer existing one is nice since it fits on one line.
Comment #6
miteshmapRemoved the old one and kept the new one. :)
Comment #7
snehi commentedwhat about this ?
Comment #8
lokapujyaThanks for all the patches. We had an initial patch and someone working on. The exact wording was under discussion. At that point, instead of posting a new patch, you should review the patch and mark it RTBC or suggest a new wording in a comment (and let the person working on it fix it.)
Anyway, I am RTBCing #6 since that it was I was looking for originally.
Comment #10
alexpottCommitted the patch from #6 as the wording change is unnecessary. @snehi I think @lokapujya has a point - I'm not sure what the point of #7 was.
Committed 408cd20 and pushed to 8.0.x and 8.1.x. Thanks!