Add API documentation to the Route process plugin.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Status: Active » Needs review
FileSize
927 bytes
phenaproxima’s picture

Assigned: Unassigned » phenaproxima

Self-assigning for review.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

phenaproxima’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/migrate/src/Plugin/migrate/process/Route.php
    @@ -11,10 +11,27 @@
    - * @link https://www.drupal.org/node/2750777 Online handbook documentation for route process plugin @endlink
    + * The route process plugin ...
    

    This seems incomplete :)

  2. +++ b/core/modules/migrate/src/Plugin/migrate/process/Route.php
    @@ -11,10 +11,27 @@
    + * The source value is an array of two values
    + *   - link_path:
    + *   - options:
    

    The first sentence needs to end with a colon, and both items in the input value need to be explained.

  3. +++ b/core/modules/migrate/src/Plugin/migrate/process/Route.php
    @@ -11,10 +11,27 @@
    + * @code
    + * process:
    + *   route:
    + *     source:
    + *       - link_path
    + *       - options
    + * @endcode
    

    We need an explanation of what this example does. And maybe another example as well, if it would be helpful.

jofitz’s picture

Assigned: phenaproxima » Unassigned
Status: Needs work » Needs review
FileSize
1.6 KB
1.63 KB

Changes in response to code review:

  1. Completed the sentence.
  2. Added the colon and explained the values.
  3. Added and explained two examples.
phenaproxima’s picture

Assigned: Unassigned » phenaproxima

Self-assigning for review.

phenaproxima’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/migrate/src/Plugin/migrate/process/Route.php
    @@ -11,10 +11,50 @@
    + * The route process plugin sets the destination route information based on the
    + * source link_path.
    

    This effectively repeats the short description. We're going to need to either explain this a lot more thoroughly (i.e., by linking to the main documentation for the D8 routing system) or remove it altogether.

  2. +++ b/core/modules/migrate/src/Plugin/migrate/process/Route.php
    @@ -11,10 +11,50 @@
    + * process:
    + *   new_route_field:
    + *     plugin: route
    + *     source:
    + *       - 'https://www.drupal.org'
    + *       -
    + *         attributes:
    + *           title: Drupal
    + * @endcode
    + *
    + * This will set new_route_field to be a route with the URL
    + * "https://www.drupal.org" and title attribute "Drupal".
    

    Can we add an example showing what will be returned if you pass an internal path like /admin/content as the source value?

jofitz’s picture

Status: Needs work » Needs review
FileSize
880 bytes
1.51 KB
  1. Removed the repeated line.
  2. Corrected the second example which should have already referenced an internal path (at least that's what I'd put in the comment).
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

I like it.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 2cfd4d7 to 8.4.x and 63ab9aa to 8.3.x. Thanks!

  • alexpott committed 2cfd4d7 on 8.4.x
    Issue #2845487 by Jo Fitzgerald, quietone, phenaproxima: Add...

  • alexpott committed 63ab9aa on 8.3.x
    Issue #2845487 by Jo Fitzgerald, quietone, phenaproxima: Add...

Status: Fixed » Closed (fixed)

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