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!lib!Drupal!Core!Entity!EntityRefe...
None of the methods in this interface document their parameters!
Comment | File | Size | Author |
---|---|---|---|
#29 | interdiff_2850057_16-29.txt | 1.57 KB | ankithashetty |
#29 | 2850057-29.patch | 1.71 KB | ankithashetty |
#25 | 2850057-applied_patch.png | 9.12 KB | Abhijith S |
#23 | Screen Shot 2020-07-29 at 7.20.20 AM.png | 353.96 KB | tanubansal |
#23 | Screen Shot 2020-07-29 at 7.20.14 AM.png | 374.48 KB | tanubansal |
Issue fork drupal-2850057
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
Pavan B S CreditAttribution: Pavan B S at Valuebound commentedUpdating the patch Please check
Comment #3
joachim CreditAttribution: joachim commentedThanks for the patch. It's a good start, but it doesn't follow our documentation standards:
For instance, each parameter needs its own @param with its own description.
Comment #4
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commentedwe are working on this in code sprint.
Comment #5
ritzz CreditAttribution: ritzz at Iksula commentedComment #6
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedNeed white space between @parm and @return, I am attaching patch for the issue, Please Review.
Comment #7
Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented@ritzz Thanks for the patch & @Munavijayalakshmi, Thanks for the merged patch! it works well.
Comment #8
xjmThanks, this is a good start to add the missing documentation. I think we need to do a little more work to come up with documentation that communicates what is needed.
The parameters should follow the format:
@param (datatype) $parameter
Optional parameters should start with
(optional)
This documentation also does not explain to me yet what the correct usage of these parameters is. Let's take a closer look at both what the method does and what code that calls it does, and try to come up with some more complete documentation.
Small correction: This should say "Ids" rather than "id's". Also, we can probably remove the word "passed".
Let's work on an updated package for those things. Be sure to look at the code in the implementations of this interface and their callers to get a good idea about what the documentation should say.
Comment #9
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commented#8 Fixed, Applying patch.
Comment #10
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedIgnore the previous patch i forgot to give solution for the condition 1 in the #8 comment. Applying the patch for all the 3 conditions please review.
Comment #11
Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented@Munavijayalakshmi, Thanks! for the updated patch,it works well.
Comment #12
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented#8.1 and #8.2 are still not addressed properly. Not all the parameters have the data type specified and the help text still doesn't describe their intended usage..
Comment #13
gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commentedComment #14
Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented@gaurav.kapoor, Thanks for the updated patch i checked your patch manually by applying it in module file & it works well after the process.
Comment #15
xjm#8.2 is still not addressed:
Thanks!
@dhwani.addweb, the automated testing infrastructure tells us whether the patch applies, so we do not need people to review that. Manual testing is not useful for API documentation patches.
What we do need people to review is whether the issue has a correct scope, whether it passes the core gates, whether the solution completely fixes the problem without introducing other problems, and whether it's the best solution we can come up with. See the patch review guide for more information.
Comment #16
gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commentedComment #23
tanubansal CreditAttribution: tanubansal at Salsa Digital commented@params are added ... Tested via #16
This can be moved to RTBC
Comment #25
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedPatch applies cleanly on 9.2.x.
Comment #26
daffie CreditAttribution: daffie commented@Abhijith: Could you do a review of the patch. The howto review can be found here: https://www.drupal.org/patch/review.
Comment #27
Vishalghyv CreditAttribution: Vishalghyv at Google Summer of Code commentedSome of the description can be improved like:
Could be rephrased as: Operator to be used for matching
Comment #28
ankithashettyWill update the changes suggested by #27 shortly. Thanks!
Comment #29
ankithashettyHere is an updated patch with an interdiff. Kindly review...
Thank you!
Comment #30
CurriedN CreditAttribution: CurriedN at Ekino commentedThanks for the patch ! Works well !
Comment #31
joachim CreditAttribution: joachim commentedThat sounds like an RTBC! Thanks for the patch and the review!
Comment #35
catchThe added docs look fine to me.
There are several comments on this issue with screenshots of the patch applying, but without actual reviews of the patch, I've removed credit for those since they don't count as reviews. See https://www.drupal.org/core/maintainers/issue-credit for more information.
Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!