Fix phpdocs in core/lib/Drupal/Core/Database/Install/Tasks.php

Hi @jyotimishra123, thanks for the well-wishing! Indeed, I'm safe and well, blessed be. Hope you're safe, too (both from the pandemic and
Cyclone Amphan (if you're anywhere near that)).

What needs to happen is to run phpcs on core/lib/Drupal/Core/Database/Install/Tasks.php:

phpcs --standard=Drupal core/lib/Drupal/Core/Database/Install/Tasks.php

FILE: ...MAMP-common/htdocs/drupal-9_1/core/lib/Drupal/Core/Database/Install/Tasks.php
-----------------------------------------------------------------------------------
FOUND 6 ERRORS AND 4 WARNINGS AFFECTING 9 LINES
-----------------------------------------------------------------------------------
 119 | ERROR   | Return type missing for @return tag in function comment
 186 | WARNING | Line exceeds 80 characters; contains 85 characters
 191 | WARNING | Only string literals should be passed to t() where possible
 194 | WARNING | Only string literals should be passed to t() where possible
 194 | ERROR   | If the line declaring an array spans longer than 80 characters,
     |         | each element should be broken into its own line
 227 | ERROR   | Missing parameter type
 230 | ERROR   | Return type missing for @return tag in function comment
 322 | ERROR   | Missing parameter type
 325 | ERROR   | Return type missing for @return tag in function comment
 345 | WARNING | Only string literals should be passed to t() where possible
-----------------------------------------------------------------------------------

and add good PHPDocs for all of these referring to comments. This patch will probably be able to be backported all the way to 8.8.x. Remember that "array" is almost never a good type for the docs, and you'll want something more specific like "string[]" (as appropriate).

The t() stuff would have to be a separate issue, since that'd be a string change and only 9.1.x at this point. Not sure if that should be a single issue, or if there's a wider effort to solve that across core that would be a more appropriate scope. You could search for something like that, and open a follow-up if not.

Not sure if "array spans longer than 80 characters..." is in scope here or not. Better to leave it out for now and focus on comment-only-changes.

Thanks!
-Derek

Cool, thanks. That's a good start!

I pulled in some of the other changes from the interdiff at #3135629-42: Minimum MySQL version requirement is not confirmed when upgrading existing sites from Drupal 8 to Drupal 9 that weren't addressed.

Also did a pass through the whole file and did some other cleanups:

- Using an active verb for each phpdoc summary (e.g. "Checks" not "Check").
- Tried to keep the summaries under 80 chars.
- Added yet more missing docs (not sure why phpcs didn't flag all of these).
...

Also, thanks to #3109534: Raise the minimum MySQL version to 5.7.8 and MariaDB version to 10.2.7 in Drupal 9, this issue now needs separate patches for 9.y.x branches vs. 8.9.x branch.

Hi @nitesh624,

At this point, neither of us are really qualified to RTBC the issue, since we've both contributed to the patch.

Your next steps could include:

A) Carefully review my changes from #12 and make sure they're all set (no typos, code standards are right, new docs are accurate, clear, concise, etc.) If you find any problems, either fix them yourself or comment here to discuss and we can decide who will fix them. You could also review the issue summary to make sure it's complete and accurate. These are things core committers appreciate before being asked to review something.

B) If #12 and the issue overall seem totally done to you, you could try to find someone else to review/RTBC this:
- Co-workers?
- Other folks you have a connection to (local user group, friends, random contributors you've worked with on other issues and have a bond, etc).
- Offer to do a "review trade" where you'll review someone else's patch and they'll review this. The #contribute and #patch-review channels on Slack can be good places to make such an offer.

That's about all you can do at this point.

Thanks!
-Derek

@jyotimishra123: Thanks for trying to move this forward. However, as I wrote at #5:

The t() stuff would have to be a separate issue, since that'd be a string change and only 9.1.x at this point. Not sure if that should be a single issue, or if there's a wider effort to solve that across core that would be a more appropriate scope. You could search for something like that, and open a follow-up if not.

So let's not worry about trying to do anything along the lines of #16. This issue isn't about trying to fix all the PHPCS errors on this file, it's about the documentation only.

Hiding the new files. Reviews / RTBC should be based on #12 for now. Re-uploading so the testbot isn't confused.

Thanks,
-Derek

If it’s a bug, let’s smash it. 😉