Problem/Motivation

Config contains a key sensor_id.
Once the renaming is landed, it becomes clear that we use the term "plugin":
#2384601: Rename Sensor to SensorPlugin

As a result, references to the plugin under "sensor_id" are even more confusing.

Proposed resolution

Rename sensor_id to plugin_id in all config.yml. (Don't forget Test and optional sensors.)
Rename SensorConfig public $sensor_id to $plugin_id.

At the same time, the sensor_id argument of TestSensorPlugin should be renamed to plugin_id.
SensorListBuilder uses the term sensor_id where i guess it refers to sensor_name. Also some type hinting here would be nice to make the context clearer.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#6 monitoring_2389041_rename_sensor_6.patch31.37 KBmiro_dietiker
#4 monitoring-2389041-rename-sensor-to-plugin-4.patch32.23 KBAnushka-mp
#4 monitoring-2389041-rename-sensor-to-plugin-4.interdiff.txt1.73 KBAnushka-mp
#1 monitoring-2389041-rename-sensor-to-plugin.patch33.96 KBAnushka-mp

Comments

Anushka-mp’s picture

Anushka-mp commented
StatusFileSize
new monitoring-2389041-rename-sensor-to-plugin.patch33.96 KB

sensor_id replaced with plugin_id, all tests are green locally.

Anushka-mp’s picture

Anushka-mp commented
Status: Active » Needs review
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work

Loooking good, except two wrong renames.

  1. +++ b/modules/multigraph/src/Form/MultigraphForm.php
@@ -25,15 +25,15 @@ class MultigraphForm extends EntityForm {
-    $sensor_ids = \Drupal::entityQuery('monitoring_sensor_config')
+    $plugin_ids = \Drupal::entityQuery('monitoring_sensor_config')

    Nope, this is Config, not Plugin, so stay with sensor_ids. 

  2. +++ b/src/SensorListBuilder.php
@@ -95,8 +95,8 @@ class SensorListBuilder extends ConfigEntityListBuilder implements FormInterface
+    foreach ($form_state->getValue('sensors') as $plugin_id => $enabled) {
+      $sensor = SensorConfig::load($plugin_id);

    Also, sensor_id remains here.

To be more clear we might want to rename the wrong renames above into other direction... Let's do this in a followup after discussing with Berdir.

Anushka-mp’s picture

Anushka-mp commented
StatusFileSize
new monitoring-2389041-rename-sensor-to-plugin-4.interdiff.txt1.73 KB
new monitoring-2389041-rename-sensor-to-plugin-4.patch32.23 KB

changes mentioned above is reverted :)

Anushka-mp’s picture

Anushka-mp commented
Status: Needs work » Needs review
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
StatusFileSize
new monitoring_2389041_rename_sensor_6.patch31.37 KB

Rerolling. Will commit when passing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Looks fine.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Committed, pushed.

  • miro_dietiker committed 84fede9 on 8.x-1.x 
    Issue #2389041 by Anushka-mp, miro_dietiker: Rename sensor_id to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.