Contrib.social

Implement module review remarks

Open on Drupal.org →
Created on 14 March 2024, 8 months ago
Updated 16 April 2024, 7 months ago

  1. Are `parameters.rdf_sync.format` from `rdf_sync.services.yml` still used somewhere? Because I see `\Drupal\rdf_sync\Model\RdfSyncFormat` being used now everywhere. 

  2.  /**
   * Checks whether the synchronization is enabled.
   *
   * @return bool
   *   TRUE if the synchronization is enabled.
   */
  public function isSynchronizationEnabled(): bool {
    return $this->state->get('rdf_sync.synchronization_enabled', TRUE);
  }  /**
   * Enables synchronization.
   *
   * @return $this
   */
  public function enableSynchronization(): self {
    $this->state->delete('rdf_sync.synchronization_enabled');
    return $this;
  }  /**
   * Disables synchronization.
   *
   * @return $this
   */
  public function disableSynchronization(): self {
    $this->state->set('rdf_sync.synchronization_enabled', FALSE);
    return $this;
  }

    The wording `synchronization_enabled` is confusing here. It would be better to be `syncrhronization_disabled` and revert the default values in the two out of the 3 methods.
    Or otherwise, to install the state value on module install, and handle only TRUE and FALSE, not delete it to enable it. 

  3.   # The rdf_sync.normalizer service is not publicly available, but we need it to
  # get a PHP array representation of entities. Create an alias for directly
  # accessing the normalizer.
  # See \Drupal\rdf_sync\RdfSyncSynchronizerTrait::doSynchronize()
  rdf_sync.normalizer:
📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇷🇴Romania claudiu.cristea Arad 🇷🇴

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024