API page: https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!EntityRefe...

None of the methods in this interface document their parameters!

CommentFileSizeAuthor
#29 interdiff_2850057_16-29.txt1.57 KBankithashetty
#29 2850057-29.patch1.71 KBankithashetty
#25 2850057-applied_patch.png9.12 KBAbhijith S
#23 Screen Shot 2020-07-29 at 7.20.20 AM.png353.96 KBtanubansal
#23 Screen Shot 2020-07-29 at 7.20.14 AM.png374.48 KBtanubansal
#16 interdiff-13-16.txt1.54 KBgaurav.kapoor
#16 addedd_individual_param_comment-2850057-16.patch1.72 KBgaurav.kapoor
#13 interdiff-10-13.txt953 bytesgaurav.kapoor
#13 addedd_individual_param_comment-2850057-13.patch1.56 KBgaurav.kapoor
#10 interdiff6_10.txt1.61 KBMunavijayalakshmi
#10 addedd_individual_param_comment-2850057-10.patch1.54 KBMunavijayalakshmi
#9 interdiff.txt1.54 KBMunavijayalakshmi
#9 addedd_individual_param_comment-2850057-9.patch1.53 KBMunavijayalakshmi
#6 addedd_individual_param_comment-2850057-6.patch1.48 KBMunavijayalakshmi
#5 addedd_individual_param_comment-2850057-5.patch1.46 KBritzz
#2 param-2850057-2.patch1.31 KBPavan B S

Issue fork drupal-2850057

Command icon 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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

Pavan B S’s picture

Pavan B S CreditAttribution: Pavan B S at Valuebound commented
Assigned: Unassigned » Pavan B S
Status: Active » Needs review
FileSize
param-2850057-2.patch1.31 KB

Updating the patch Please check

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Needs work

Thanks for the patch. It's a good start, but it doesn't follow our documentation standards:

+++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionInterface.php
@@ -16,27 +16,27 @@
-   * @return array
+   * @param $match, $match_operator, $limit

For instance, each parameter needs its own @param with its own description.

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Issue tags: +DrupalMumbaiCodeSprint16

we are working on this in code sprint.

ritzz’s picture

ritzz CreditAttribution: ritzz at Iksula commented
Assigned: Pavan B S » ritzz
Status: Needs work » Needs review
FileSize
addedd_individual_param_comment-2850057-5.patch1.46 KB
Munavijayalakshmi’s picture

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commented
Assigned: ritzz » Munavijayalakshmi
FileSize
addedd_individual_param_comment-2850057-6.patch1.48 KB

Need white space between @parm and @return, I am attaching patch for the issue, Please Review.

Aanal.addweb’s picture

Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs review » Reviewed & tested by the community

@ritzz Thanks for the patch & @Munavijayalakshmi, Thanks for the merged patch! it works well.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work

Thanks, 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.

  1. +++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionInterface.php
@@ -17,6 +17,13 @@
+   * @param $match

    The parameters should follow the format:
    @param (datatype) $parameter 

  2. +++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionInterface.php
@@ -17,6 +17,13 @@
+   *    String that needs to be matched.
...
+   *    Match operator that needs to be applied on String.
...
+   *    Limit which is set to 0 as default.

@@ -27,6 +34,11 @@ public function getReferenceableEntities($match = NULL, $match_operator = 'CONTA
+   *    String that needs to be matched.
...
+   *    Match operator that needs to be applied on String.

    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. 

  3. +++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionInterface.php
@@ -35,6 +47,9 @@ public function countReferenceableEntities($match = NULL, $match_operator = 'CON
+   *   An array of id's passed to validate.

    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.

Munavijayalakshmi’s picture

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commented
Assigned: Munavijayalakshmi » Unassigned
Status: Needs work » Needs review
FileSize
addedd_individual_param_comment-2850057-9.patch1.53 KB
interdiff.txt1.54 KB

#8 Fixed, Applying patch.

Munavijayalakshmi’s picture

Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commented
FileSize
addedd_individual_param_comment-2850057-10.patch1.54 KB
interdiff6_10.txt1.61 KB

Ignore 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.

Aanal.addweb’s picture

Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs review » Reviewed & tested by the community

@Munavijayalakshmi, Thanks! for the updated patch,it works well.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Reviewed & tested by the community » Needs work

#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..

gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented
Status: Needs work » Needs review
FileSize
addedd_individual_param_comment-2850057-13.patch1.56 KB
interdiff-10-13.txt953 bytes
Aanal.addweb’s picture

Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs review » Reviewed & tested by the community

@gaurav.kapoor, Thanks for the updated patch i checked your patch manually by applying it in module file & it works well after the process.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work

#8.2 is still not addressed:

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.

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.

gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented
Status: Needs work » Needs review
FileSize
addedd_individual_param_comment-2850057-16.patch1.72 KB
interdiff-13-16.txt1.54 KB

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

tanubansal’s picture

tanubansal CreditAttribution: tanubansal at Salsa Digital commented
FileSize
Screen Shot 2020-07-29 at 7.20.14 AM.png374.48 KB
Screen Shot 2020-07-29 at 7.20.20 AM.png353.96 KB

@params are added ... Tested via #16
This can be moved to RTBC

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Abhijith S’s picture

Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
FileSize
2850057-applied_patch.png9.12 KB

Patch applies cleanly on 9.2.x.

after

daffie’s picture

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.

Vishalghyv’s picture

Vishalghyv CreditAttribution: Vishalghyv at Google Summer of Code commented
Status: Needs review » Needs work

Some of the description can be improved like:

The operation the matching should be done with.

Could be rephrased as: Operator to be used for matching

ankithashetty’s picture

ankithashetty
Location Karnataka, India
CreditAttribution: ankithashetty as a volunteer and at Specbee commented
Assigned: Unassigned » ankithashetty

Will update the changes suggested by #27 shortly. Thanks!

ankithashetty’s picture

ankithashetty
Location Karnataka, India
CreditAttribution: ankithashetty as a volunteer and at Specbee commented
Assigned: ankithashetty » Unassigned
Status: Needs work » Needs review
FileSize
2850057-29.patch1.71 KB
interdiff_2850057_16-29.txt1.57 KB

Here is an updated patch with an interdiff. Kindly review...

Thank you!

CurriedN’s picture

CurriedN CreditAttribution: CurriedN at Ekino commented

Thanks for the patch ! Works well !

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Reviewed & tested by the community

That sounds like an RTBC! Thanks for the patch and the review!

init90 made their first commit to this issue’s fork.

  • catch committed b16784b on 9.2.x 
    Issue #2850057 by Munavijayalakshmi, gaurav.kapoor, ankithashetty, Pavan...

  • catch committed 79737c3 on 9.1.x 
    Issue #2850057 by Munavijayalakshmi, gaurav.kapoor, ankithashetty, Pavan...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

The 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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.