Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.
@aditya_anurag, I saw your patch and noticed some points to improve.
- Docblocks need a blank line between the first line such as * Implements hook_install_tasks() and the rest.
- Docblocks also need a blank line between @param group and @return.
- The descriptions for @param and @return are not inconsistent in the code. If you want to improve the docblock, Isn't it better to add descriptions to all of new @param and @return?
- @internal may not be necessary.
Please try seeing the documentation standard again.
This also appears to do some trailing whitespace fixing, which I guess is fine since we don't have (that I'm aware of) other patches working on this same code. Usually, we like to isolate whitespace fixes to their own issues because they break other patches.
Comments
Comment #2
aditya_anurag commentedAdded a patch for this.
Comment #3
aditya_anurag commentedComment #4
aditya_anurag commentedComment #5
hgoto commented@aditya_anurag, I saw your patch and noticed some points to improve.
- Docblocks need a blank line between the first line such as
* Implements hook_install_tasks()and the rest.- Docblocks also need a blank line between @param group and @return.
- The descriptions for @param and @return are not inconsistent in the code. If you want to improve the docblock, Isn't it better to add descriptions to all of new @param and @return?
- @internal may not be necessary.
Please try seeing the documentation standard again.
https://www.drupal.org/coding-standards/docs
Comment #6
rasikap commentedComment #7
rasikap commentedAdded patch for this issue. Also fixed some coding standard issues.
Comment #8
rasikap commentedComment #9
dsnopekThanks for the contribution!
This also appears to do some trailing whitespace fixing, which I guess is fine since we don't have (that I'm aware of) other patches working on this same code. Usually, we like to isolate whitespace fixes to their own issues because they break other patches.
One small bit of review:
This looks like an unintentional addition?
Comment #10
rasikap commentedComment #11
rasikap commentedHi dsnopek,
So I would just need to remove the unintentional addition correct? or is there anything more that needs to be changed?
Comment #12
dsnopekYes, please! Thanks in advance :-)
Comment #13
rasikap commentedAdded patch after removing unintentional addition.
Comment #14
rasikap commentedComment #15
rasikap commentedComment #16
Christie Alcidor commented