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.Problem/Motivation
API page: https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Row.p...
> array An array of source plugins.
The return is not an array of plugins, but the array of source values.
Steps to reproduce
N/A
Proposed resolution
Change method short comment to "Retrieves all source properties." To be consistent with method ->getSourceProperty()
Change @return description to "An array containing all source property values, keyed by the property name."
Remaining tasks
Commit
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
Issue fork drupal-3251835
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:
changes
Comments
Comment #2
beatrizrodriguesI'll fix the @return doc.
Comment #4
beatrizrodriguesI did the MR, I hope it fits. Thanks
Comment #5
joachim CreditAttribution: joachim at Factorial GmbH commentedThanks for the patch!
LGTM; I'll ask the migrate maintainer to confirm.
Comment #6
mikelutzMost other places in the Row class documentation, we refer to source 'properties' rather than values. Let's go with "An array containing all source property values, keyed by the property name."
Comment #7
mikelutzLets tweak *Returns the whole source array." to "Retrieves all source properties." while we are in there, to more closely align with the 'getSourceProperty' method above it.
Comment #8
beatrizrodriguesI did the fixes that you reported @mikelutz Thank you so much for the feedback.
Comment #9
mikelutzComment #11
beatrizrodriguesohh! I didn't know that @mikelutz, I was actually in doubt about where should do the split in the test. Thank you for share this information with me. I'll fix that.
Comment #12
beatrizrodriguesSplited the line in the place that was suggested. I run phpcs and everything looks fine. Please, Review. Thanks
Comment #13
joachim CreditAttribution: joachim as a volunteer commentedLooks perfect! Thanks everyone!
> I was actually in doubt about where should do the split in the test.
BTW, most text editors and IDEs should have a function to automatically rewrap comment text for you. E.g. for VSCode there's the rewrap plugin.
Comment #14
mikelutzMuch better. Thanks!
Tweaking the issue description and tags. For future reference, this issue should be in the migration component with a tag of documentation to get the attention of migration maintaiers, instead of being in the documentation component and being tagged as 'migrate'. The migrate tag is not a standard one we use or look for, and the documentation component is really more for online documentation change requests, rather than in code comment and phpdoc changes.
Comment #15
beatrizrodriguesOh, thanks @joachim. I will use it from now. I was not aware of this plugin existence.
Comment #16
joachim CreditAttribution: joachim as a volunteer commented> The migrate tag is not a standard one we use or look for,
I'd previously seen someone from the migrate team (possibly @quietone?) tag a documentation issue with this tag. At least I think it was this tag. It was definitely a tag!
> and the documentation component is really more for online documentation change requests, rather than in code comment and phpdoc changes.
That's not correct at all. If you look at an api.d.o page such as https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Row.p... and follow the 'file an issue' link, it takes you to a new issue form and prefills the 'component' as 'documentation'. The 'documentation' tag is, AFAIK a relic from years ago and unless the documentation team have changed their guidelines, should not be used.
Comment #17
mikelutz> That's not correct at all. If you look at an api.d.o page such as https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Row.p... and follow the 'file an issue' link, it takes you to a new issue form and prefills the 'component' as 'documentation'.
Yes, filing an issue on any documentation page will default to the 'documentation' component, but as I said, that is not correct for this issue.
> I'd previously seen someone from the migrate team (possibly @quietone?) tag a documentation issue with this tag. At least I think it was this tag. It was definitely a tag!
While I can't speak to that, I can assure you that as we look through the migration issue queue, and tally up statistics, we use the migration component filter, not tags. This issue is correctly placed in the migration system component.
Comment #18
quietone CreditAttribution: quietone at PreviousNext commentedIt is very likely me adding the documentation tag and moving issues to the migration system because that is where the migrate maintainers work.
As I understand Selecting the Project for a documentation issue issue should be in the documentation component. But then my question is 'how do sub-system maintainers find documentation issues about their component(s)'? Presumably one adds a tag for the component and then maintainers need to do, at least, two searches to find everything related to their sub-system. In some ways, that is fine. I just would like to know what is expected, they are grey areas here.
Comment #19
quietone CreditAttribution: quietone at PreviousNext commentedHopefully improving the title by removing '[documentation]' since there is 'docs' later on.
Comment #20
alexpottCommitted and pushed 2a68e8f22c4 to 10.0.x and dbca2fc0e2b to 9.4.x and 90bd6795ae7 to 9.3.x. Thanks!