Remove the left-over @todo comments in the BaseFieldDefinition class

Hi,

if I have understood this correctly the unnecessary code has to be removed as well as the @todo comment.

Patch https://www.drupal.org/files/issues/d8_definition_defaults_3.patch in https://www.drupal.org/node/2116341 adds in the functionality that is currently in the function we are talking about. See:

  protected function getFieldItemClass() {
    if ($class = $this->getItemDefinition()->getClass()) {
      return $class;
    }
    else {
      $type_definition = \Drupal::typedDataManager()
        ->getDefinition($this->getItemDefinition()->getDataType());
      return $type_definition['class'];
    }
  }

vs.

  public function getClass() {
    if (isset($this->definition['class'])) {
      return $this->definition['class'];
    }
    else {
      $type_definition = \Drupal::typedDataManager()->getDefinition($this->getDataType());
      return $type_definition['class'];
    }
  }

I have attached patch removing the duplicate code.

Yes this was my thinking also with this. Better to remove the whole function.

BaseFieldDefinition is public API, so I don't think we can get away with removing the protected method anymore following the release of 8.0.0. What we can do instead though is remove the usages of it and instead deprecate it. Reference: https://www.drupal.org/core/deprecation

Thanks!

@kiamlaluno, see: https://www.drupal.org/core/deprecation#internal

So, again, let's remove the usages and deprecate the method instead of removing it. Thanks!

I was thinking the same thing should we remove the function or no. But this final patch seems to do the job and applies as it should.

Thanks, #17 looks good! We need just one more thing, a @trigger_error() inside the function so that it gets removed. Example here: https://www.drupal.org/core/deprecation#how-method

+++ b/core/lib/Drupal/Core/Field/BaseFieldDefinition.php
@@ -641,15 +640,15 @@ public function getPropertyNames() {
+   * Use \Drupal\Core\TypedData\ListDataDefinition::getClass() instead.

Small thing, this docs line should be indented by two spaces.

Good catch. Also this trigger_error is new to me. Added trigger_error & added two spaces.

Okay, that looks good! We don't need a change record for this one since it was only ever an internal helper; the deprecation notice by itself is sufficient in this case.