Move DateTimeItem and DateRangeItem constants onto interfaces.

I will work on this issue.

Here is my patch, please review it:)

I checked the reports but I don't know why the 'FieldTypePluginManagerTest' class fails.
Here is the new patch with some updated.
Thanks.

With the latest test-bot results, i think the patch is looks good and even test is passed.
Changing the status from need work to need review.
Re-run the tests to finalizing the patch.

thanks:)

I am checking:)

Idead, it was this problem. Thanks

Let's update the existing one.

Global constants in datetime.module are deprecated and DateTimeItemInterface has been introduced.

Patch for version 8.6.x , Please review it :)

Re-roll of patch in #15 (with interdiff against patch in #23 for reference).

Patch #29 looks good to me. Marking RTBC.

This should work fine :)

+++ b/core/modules/datetime/tests/src/Kernel/Views/DateTimeHandlerTestBase.php
@@ -3,7 +3,7 @@
 use Drupal\datetime\Plugin\Field\FieldType\DateTimeItemInterface;

This patch fails because of this duplicate use statement. Here is updated patch.

Patch updated with some references that were missing and added missing interface.

One more try

Looks good to me marking RTBC.

We still have to update change record.

Here is updated patch which removes all the DateRangeItem:: constant mentioned in #44 and unused use statements.

Patch #46 looks good to me. Marking RTBC.

Testbot had a hiccup re-rtbcing

Address @mpdonadio's comments in #51.

The patch #52 applied successfully and takes all points into account as stated by @mpdonadio in #51.
Thanks @JoFitzgerald for the patch.
Marking it RTBC.

+++ b/sites/default/default.settings.php
@@ -573,10 +573,10 @@
-# $settings['locale_custom_strings_en'][''] = [
+# $settings['locale_custom_strings_en'][''] = array(
...
-# ];
+# );
 

@@ -630,7 +630,7 @@
-# $settings['bootstrap_config_storage'] = ['Drupal\Core\Config\BootstrapConfigStorageFactory', 'getFileStorage'];
+# $settings['bootstrap_config_storage'] = array('Drupal\Core\Config\BootstrapConfigStorageFactory', 'getFileStorage');

These are unexpected changes.

Fixing the issues mentioned by @alexpott. Even all the changes in default.settings.php is unexpected which convert square [] array syntax to array() syntax.

Here is updated patch which revert the default.settnigs.php changes. I am not sure if @Jo Fitzgerald's intention of making these changes was to make the uniform syntax of array in this file. Currently default.settings.php file uses mixture of square [] syntax and array() syntax which is not a good idea. Fixing this kind of inconsistency as a side effect in this issue is also not a good idea. Converting array syntax to square array syntax should be followed up on this #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase

The latest patch addresses the remaining feedback I think.

1. +++ b/core/modules/datetime_range/src/Plugin/Field/FieldWidget/DateRangeDatelistWidget.php
   @@ -4,7 +4,7 @@
   +use Drupal\datetime_range\Plugin\Field\FieldType\DateRangeItemInterface;
   
   @@ -38,7 +38,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   +    if ($this->getFieldSetting('datetime_type') == DateRangeItemInterface::DATETIME_TYPE_DATETIME) {
   
   @@ -103,7 +103,7 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
   +    if ($this->getFieldSetting('datetime_type') == DateRangeItemInterface::DATETIME_TYPE_DATETIME) {
   
   @@ -146,7 +146,7 @@ public function settingsSummary() {
   +    if ($this->getFieldSetting('datetime_type') == DateRangeItemInterface::DATETIME_TYPE_DATETIME) {
   

   Should these refer to the parent DateTimeInterface instead of the DateTimeRangeInterface?

   DateTimeRangeInterface should only be used for the DATETIME_TYPE_ALLDAY constant right?

2. +++ b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php
   @@ -57,7 +57,7 @@ public function testDateRangeField() {
   +      $this->fieldStorage->setSetting('datetime_type', DateRangeItemInterface::DATETIME_TYPE_DATE);
   
   @@ -280,7 +280,7 @@ public function testDatetimeRangeField() {
   +    $this->fieldStorage->setSetting('datetime_type', DateRangeItemInterface::DATETIME_TYPE_DATETIME);
   
   @@ -624,7 +624,7 @@ public function testDatelistWidget() {
   +    $this->fieldStorage->setSetting('datetime_type', DateRangeItemInterface::DATETIME_TYPE_DATE);
   
   @@ -695,7 +695,7 @@ public function testDatelistWidget() {
   +    $this->fieldStorage->setSetting('datetime_type', DateRangeItemInterface::DATETIME_TYPE_DATETIME);
   
   @@ -983,7 +983,7 @@ public function testDefaultValue() {
   +      'settings' => ['datetime_type' => DateRangeItemInterface::DATETIME_TYPE_DATE],
   
   @@ -1142,7 +1142,7 @@ public function testDefaultValue() {
   +    $this->fieldStorage->setSetting('datetime_type', DateRangeItemInterface::DATETIME_TYPE_DATETIME);
   
   @@ -1350,7 +1350,7 @@ public function testDateStorageSettings() {
   +        'datetime_type' => DateRangeItemInterface::DATETIME_TYPE_DATE,
   

   Few other instances

3. +++ b/core/modules/datetime_range/tests/src/Kernel/DateRangeItemTest.php
   @@ -50,7 +50,7 @@ protected function setUp() {
   -      'settings' => ['datetime_type' => DateRangeItem::DATETIME_TYPE_DATE],
   +      'settings' => ['datetime_type' => DateTimeItemInterface::DATETIME_TYPE_DATE],
   

   This looks right, but in other places we've done it with the DateTimeRangeInterface - I think we should be consistent, and use the defining interface, not the parent

@larowlan Thanks for review. In point 1. and 2. all the places you mentioned we are using DateRangeItemInterface::<CONSTANT> only where it is related to daterange field like in DateRangeDatelistWidget.php and DateRangeFieldTest.php and DateRangeItemInterface extends DateTimeItemInterface. So i think it is safe to use DateRangeItemInterface::<CONSTANT> for daterange field. I am not sure if there is any downside to this.

I agree with you on point 3 we should use DateTimeRangeInterface instead of DateTimeItemInterface because that test is related to DateRangeItemTest.

+++ b/core/modules/datetime_range/tests/src/Kernel/DateRangeItemTest.php
@@ -50,7 +50,7 @@ protected function setUp() {
+      'settings' => ['datetime_type' => DateTimeItemInterface::DATETIME_TYPE_DATE],

@@ -82,7 +82,7 @@ protected function setUp() {
-    $this->fieldStorage->setSetting('datetime_type', DateRangeItem::DATETIME_TYPE_DATE);

This same goes for line 86 as well.

MR seems okay. The last fail is random and unrelated. Changing status to Needs Review.

D10 version needed
At this time we need a D10.1.x patch or MR for this issue.

Patch or MR doesn't apply anymore
The last patch or MR doesn't apply to the target branch, please reroll the code so that it can be reviewed by the automated testbot.

Re Rolled for MR #2321 to 10.1.x branch

Was tagged for change record updates. Maybe this needs it's own now.

Draft CR Created.

Line core/modules/datetime/tests/src/Kernel/Views/DateTimeHandlerTestBase.php
------ ----------------------------------------------------------------------------------
72 Access to an undefined static property
static(Drupal\Tests\datetime\Kernel\Views\DateTimeHandlerTestBase)::$field_type.
------ ----------------------------------------------------------------------------------

------ -------------------------------------------------------------------------
Line core/modules/datetime/tests/src/Kernel/Views/FilterDateTest.php
------ -------------------------------------------------------------------------
53 Access to an undefined static property
static(Drupal\Tests\datetime\Kernel\Views\FilterDateTest)::$field_name.
------ -------------------------------------------------------------------------

------ -------------------------------------------------------------------------------
Line core/modules/datetime_range/tests/src/Kernel/Views/FilterDateTest.php
------ -------------------------------------------------------------------------------
60 Access to an undefined static property
static(Drupal\Tests\datetime_range\Kernel\Views\FilterDateTest)::$field_name.

Addressed the CCF error as described in #86.

@sahil.goyal changes to variable name format should be in other way around.

Moving to NW for failures in the MR.

Searching for DateTimeItem:: still see there are about 30+ instances remaining.

Please review.

Will let the committers decide but there are still some instances of DateTimeItem:: but they are in comments for some. Should these be updated too? Didn't think it was worth holding up the ticket.

  /**
   * Tests DateTimeItem::setValue().
   */

grep -R --color "DateTimeItem::" ./core
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: * Tests DateTimeItem::setValue().
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: // Test DateTimeItem::setValue() using string.
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: $this->assertEquals($value, $entity->field_datetime[0]->value, 'DateTimeItem::setValue() works with string value.');
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: // Test DateTimeItem::setValue() using property array.
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: $this->assertEquals($value, $entity->field_datetime[0]->value, 'DateTimeItem::setValue() works with array value.');
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: // Test DateTimeItem::setValue() using string.
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: $this->assertEquals($value, $entity->field_datetime[0]->value, 'DateTimeItem::setValue() works with string value.');
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: // Test DateTimeItem::setValue() using property array.
./core/modules/datetime/tests/src/Kernel/DateTimeItemTest.php: $this->assertEquals($value, $entity->field_datetime[0]->value, 'DateTimeItem::setValue() works with array value.');
./core/modules/serialization/tests/src/Unit/Normalizer/DateTimeIso8601NormalizerTest.php: if ($parent_field_item_class === DateTimeItem::class) {
./core/modules/serialization/tests/src/Unit/Normalizer/DateTimeIso8601NormalizerTest.php: if ($parent_field_item_class === DateTimeItem::class) {
./core/modules/serialization/tests/src/Unit/Normalizer/DateTimeIso8601NormalizerTest.php: DateTimeItem::class,
./core/modules/serialization/tests/src/Unit/Normalizer/DateTimeIso8601NormalizerTest.php: DateTimeItem::class,

Please ignore Merge request !2321 ,

Updated Merge request !3013 to replace existing few more instances of "DateTimeItem::"