Document the values of FieldTimeIntervalTest::$ages

Added doc comments for the values of the array $ages in FieldTimeIntervalTest. Patch attached. Marked as needs review.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

pooja saraah → made their first commit to this issue’s fork.

shivam-kumar → closed merge request !6222

Closed MR MR #6222 as new MR MR #6223 Is created for the same.

Hi @shivam-kumar
Applied MR!6223 and it looks good but getting warnings ( whitespace errors ) please have a look.

core/6223.patch.txt:18: trailing whitespace.
   * 
core/6223.patch.txt:25: trailing whitespace.
   * 
Checking patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php...
Checking patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php...
Checking patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php...
Checking patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php...
Applied patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php cleanly.
Applied patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php cleanly.
Applied patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php cleanly.
Applied patch core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php cleanly.
warning: 2 lines add whitespace errors.

Thanks

Hi @shweta__sharma Thanks for pointing, however, the mentioned lines (18 and 25) haven't been touched. It is same as it was earlier. This is out of the scope of this issue, hence moving it to needs review.

Issue was tagged for novice and looking at @shivam-kumar post history that seems okay, would say would you get to 4 pages of issues could work on non-novice issues. But side note I couldn't help but notice a lot of the posts were those PHPCS that have been reported. Would refrain from those here's some useful documentation just in case

Resource #1: A video introduction to contribution:
https://www.youtube.com/watch?v=lu7ND0JT-8A
Resource #2: A slide deck which goes into greater depth about contribution:
https://docs.google.com/presentation/d/1jvU0-9Fd4p1Bla67x9rGALyE7anmzjhQ...
Resource #3: The First Time Contributors Workshop from DrupalCon Global:
https://www.youtube.com/watch?v=0K0uIgKaVNQ
Resource #4: Issue Etiquette
https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... →
Resource #5: Credit Abuse Policy
https://www.drupal.org/drupalorg/docs/marketplace/abuse-of-the-contribut... →

MR 6233 seems fine though

The first value can also be NULL, however the index0 arg doc does not describe the why this is, or that it is possible.

Any thoughts for properly typing this property? @var or @phpstan-var array<int|null, string, int>

This doesnt look like a bug.

Updated issue summary and title a bit.

Hi there! I just ran PHPCS around the updated file and I got some feedback.

I ran this in LDE:
$ ddev exec phpcs --standard="Drupal,DrupalPractice" ./core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php

As feedback:

FILE: /var/www/html/core/modules/views/tests/src/Kernel/Handler/FieldTimeIntervalTest.php
-----------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------
 37 | WARNING | Line exceeds 80 characters; contains 90 characters
 38 | WARNING | Line exceeds 80 characters; contains 109 characters
-----------------------------------------------------------------------------------------
Time: 91ms; Memory: 6MB

I've created a patch and added it to this comment. I have also added the change as a new commit to the fork created for the issue, working branch "issue-3414947-document-the-values".

BTW, I'm facing some problems with pipelines, specifically running Nightwatch (you can see the error here: https://git.drupalcode.org/issue/drupal-3414947/-/jobs/699582). It seems to be a recurring issue and not related to the changes applied here: https://www.drupal.org/project/drupal/issues/2829040 🌱 [meta] Known intermittent, random, and environment-specific test failures Active

Greetings.

Look good to me. RTBC

Thanks for working on this improvement!

I don't recall seeing "index 0" for describing an array in core, so I did some searching. I found these which have relevant examples.

• class docs for \Drupal\language\Plugin\migrate\process\ContentTranslationEnabledSetting
• class docs for \Drupal\Tests\migrate\Kernel\MigrateSourceTestBase::providerSource
• method docs for \Drupal\Tests\Core\Database\ConnectionTest::providerEscapeTables

Of those, MigrateSourceTestBase is likely the best one to use as a guide for this issue. I suggest the this is reworked in that style.

Also, it is worth getting familiar with the Drupal coding standards → . This issue is for comments and most of that is covered in API documentation and comment standards → . There is a lot of information in those documents and takes time and practice to use them.