Problem/Motivation

Process plugin documentation is being moved from the Handbook to the API documentation of the process plugin class.

The handbook page for process plugin contains more information than the API documentation of the plugin.

Before we delete the handbook page and redirect it to the API page, we need to either enhance the API doc or decide that it is OK to discard the additional information currently found in the handbook.

The differencec are these:
--clips--

  • The text format example has a static_map in the handbook example. This is missing from the API doc.
  • The handbook page has an attachment with a complete input and output example.

Proposed resolution

Copy the more comprehensive example to the API doc.
To be considered: do we want to include the full input / output example in the API doc?

Remaining tasks

Do it.

User interface changes

N/A.

API changes

N/A.

Data model changes

N/A.

CommentFileSizeAuthor
#12 interdiff-29337762-2-12.txt1.31 KBmasipila
#12 29337762-12.patch3.78 KBmasipila
#2 2933776-2.patch3.79 KBmasipila
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

masipila created an issue. See original summary.

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented
Status: Active » Needs review
FileSize
2933776-2.patch3.79 KB

Initial patch attached.

I copied the static_map example from the documentation handbook to the API documentation and did some minor clarification to improve the readability at the same time. I also added '@see' references to migration_lookup and static_map process plugins that are used in the examples.

What comes to the complete input and output example that was in the attachment of the handbook page: I don't consider that to be needed. The examples should now IMO be sufficient with the additional clarifications.

Markus

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Needs review » Reviewed & tested by the community

Docs look like an improvement. LGTM.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

@masipila, when this is committed the handbook page will be deleted like the other process plugins?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 8.4.x-dev » 8.5.x-dev

8.4.x will only receive critical backports now

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented

Re: #4. Correct.

Once this is committed I will:

1) Update the current handbook page with a disclaimer saying that the content has moved to API docs and link to the relevant page in API.

2) Hide the handbook page from handbook navigation.

Once all the issues of the parent meta have landed (after this week's sprint I hope), I'll request d.o. doc infra team to make redirects from the original (hidden) handbook pages to the the relevant page in api.drupal.org. I'll do this as one request to save their time.

Markus

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

@masipila, thanks, it is very clear what is to happen.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented 
+++ b/core/modules/migrate/src/Plugin/migrate/process/SubProcess.php
@@ -18,8 +18,10 @@
+ * for the following source data.

@@ -58,16 +58,17 @@
+ * static_map process plugin can be applied for the following source data.

Do we say "applied for the following source data" or should it be "applied on the following source data"? This may be a wording pattern used elsewhere. I grepped the source but did not find "applied for" or "applied on" either in core in migrate API docs.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Issue tags: +Migrate January 2018 Sprint
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Novice

No pattern here. Back to NW for #8. And tagging Novice.

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented

Applied 'to' and 'on' seem to be proper English. If I interpret this correctly, we should use applied 'on' in this context. https://english.stackexchange.com/questions/202196/apply-on-or-apply-to

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented
Status: Needs work » Needs review
FileSize
29337762-12.patch3.78 KB
interdiff-29337762-2-12.txt1.31 KB

Now we're using 'applied on' consistently in this.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Having read the link about applied on and applied to, I agree with masipila that 'applied on' makes sense in this case. However, it is pointed out in #8 that this phrasing is not used elsewhere in core and since I am a big fan of consistency I wonder if this should be changed to some other phrasing. To help answer my own question I installed the patch and reread the examples. I found it easy to understand and the 'applied on' phrasing served to distinguish that this is not a typical process plugin where the input data is transformed. So, yes it works for me and hopefully others as well. And even better now that 'applied on' is used consistently throughout.

Setting to RTBC since I presume tests will pass.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Reviewed & tested by the community » Fixed

Superb, thanks for the update and for broadening my English knowledge as well.

  • Gábor Hojtsy committed ed35f80 on 8.5.x 
    Issue #2933776 by masipila, quietone: Merge handbook documentation to...

  • Gábor Hojtsy committed a8de786 on 8.6.x 
    Issue #2933776 by masipila, quietone: Merge handbook documentation to...

Status: Fixed » Closed (fixed)

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