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.Some functions in features don't have sufficient or accurate doc comments.
We want accurate type information on @param and @return docs, so that IDEs can assist with code verification.
We also want good explanations to improve DX.
Better compliance with Drupal coding standards is also a criterion, but the major goal is DX.
Feature branch
See https://github.com/donquixote/drupal-features/compare/issue-3060511-7x2x...
Patches in this issue are only for testbot.
EDIT: After pm with @mpotter, we decided to commit this as one big change. Local development happens in a feature branch, but in the end it is all squashed.
Scope
The goal is for all (exceptions below) function docs to
- have the most accurate type information possible for IDEs to consume
- have accurate descriptions on the function itself and its parameters
- any modified doc comment should be compliant with DCS, as far as it makes sense
A few minor changes in other docs outside of the function docs are included as well.
Not included:
- Anything that would change the code itself, instead of just the docs.
- Docs on hook implementations. These are dealt with in their own dedicated issue.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | features-7.x-2.x-3060511-15-vs-10.interdiff.txt | 75.52 KB | donquixote |
| #15 | features-7.x-2.x-3060511-15-docs.patch | 131.54 KB | donquixote |
| |||
Comments
Comment #2
donquixote CreditAttribution: donquixote as a volunteer commentedI suggest to apply this patch with "git am" or "git am --signoff" to get all the commit messages.
Comment #5
donquixote CreditAttribution: donquixote as a volunteer commentedUPDATE
A lot more is coming.
I just need to do some fine-tuning, and group the commits. 147+ commits is a bit much :)
Comment #6
donquixote CreditAttribution: donquixote as a volunteer commentedMoar better!
I have this as a branch with many commits, but I am uploading it as a single patch to hit the testbot.
Comment #7
donquixote CreditAttribution: donquixote as a volunteer commentedComment #8
donquixote CreditAttribution: donquixote as a volunteer commentedI used the wrong diff format :)
Comment #9
donquixote CreditAttribution: donquixote as a volunteer commentedI might do something about this..
The main goal is DX, not compliance. But if I am already there, I should do it in a way that increases compliance.
Comment #10
donquixote CreditAttribution: donquixote as a volunteer commentedAnother attempt, more compliant with Drupal CS.
I do not attempt full compliance, there is just too much in the code that I don't want to touch. I just want my own changes to not introduce new CS problems.
If some still slip through, that's ok, we will survive it.
Comment #11
donquixote CreditAttribution: donquixote as a volunteer commentedComment #12
donquixote CreditAttribution: donquixote as a volunteer commented:(
I could use phpcbf, but I don't want so much disruption and scope creep in the entire files. I only want to fix those places where I modified the doc.
I will find a solution :)
Comment #13
donquixote CreditAttribution: donquixote as a volunteer commentedLooks like this is mostly missing @param doc text, for parameters that were not documented at all before.
I have three options:
1. Do NOT add the @param docs. -> bad.
2. Do add the @param docs, but no doc text, unless/until I have something to say. -> bad for compliance, but good for DX and for the IDE, and easy to do.
3. Do add the @param docs with poor quality doc text. -> risk of useless or misleading text.
4. Do add the @param docs with accurate doc text. -> risk to waste a lot of time.
In my own projects I always opt for option 2, this gives me the option to upgrade later.
I think I am going for this as well here.
Comment #14
donquixote CreditAttribution: donquixote as a volunteer commentedThis time I think phpcs will be happier.
I created a variation of the Drupal CS that only checks docs.
Some existing CS issues in docs will be dealt with in follow-up issues. Especially everything in hooks implementations.
At least this way I don't introduce CS regressions.
Comment #15
donquixote CreditAttribution: donquixote as a volunteer commentedDiff format..
Comment #16
donquixote CreditAttribution: donquixote as a volunteer commentedThis seems acceptable to me!
Comment #17
donquixote CreditAttribution: donquixote as a volunteer commentedComment #19
donquixote CreditAttribution: donquixote as a volunteer commentedCommitted. Let's move on!
If anyone finds objections with some of the changes, open a new issue!
Comment #21
donquixote CreditAttribution: donquixote as a volunteer commentedI already found one mistake, which will need a follow-up.
On _drush_features_component_filter() we now have a garbled param comment text on the $options parameter.