Problem/Motivation

getProcessPlugins() is named, and documented, to say you call it to get process plugins.

However, it has a $process parameter which is optional, but which isn't documented as being optional, and doesn't explain what happens when it's omitted:

   * @param array $process
   *   A process configuration array.

  public function getProcessPlugins(?array $process = NULL);

        $pipeline = $this->migration->getProcessPlugins();

Steps to reproduce

N/A

Proposed resolution

Update documentation

Remaining tasks

Review

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#18 issues_3458295_18.png48.92 KBkleiton_rodrigues

Issue fork drupal-3458295

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joachim created an issue. See original summary.

quietone’s picture

quietone commented
Version: 11.0.x-dev » 11.x-dev

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Prem Suthar made their first commit to this issue’s fork.

Prem Suthar opened merge request !8769

pooja_sharma made their first commit to this issue’s fork.

pooja_sharma’s picture

pooja_sharma commented
Status: Active » Needs review

Rebased the MR with latest code & checked, if passed NULL or omitted parameter in getProcessPlugins() then return empty [], on the basis of it , updated doc comments & applied suggestions.

PLease review, moving NR.

smustgrave’s picture

smustgrave commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Seems straight forward and reads fine

joachim’s picture

joachim commented
Status: Reviewed & tested by the community » Needs work

Doesn't make sense though!

pooja_sharma’s picture

pooja_sharma commented
Status: Needs work » Needs review

Address feedback, Please review moving NR

joachim’s picture

joachim commented
Status: Needs review » Needs work

> Th method returns an empty array if no process plugins are specified
* in the migration's process configuration.

That isn't it either.

If you specify $process, then you get the plugins from the given process array.

If you don't specify $process, then you get the plugins from the migration definition's process array.

pooja_sharma’s picture

pooja_sharma commented
Status: Needs work » Needs review

There was some misinterpretation with prev feedback from my end, tried to apply changes on the basis latest detail suggestion

Please review, moving to NR

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

1 open thread.

shalini_jha made their first commit to this issue’s fork.

shalini_jha’s picture

shalini_jha commented
Status: Needs work » Needs review

I have addressed the feedback related to the documentation comments. Please review the changes.

joachim’s picture

joachim commented
Status: Needs review » Needs work

Nearly there! Just one small fix needed.

shalini_jha’s picture

shalini_jha commented
Status: Needs work » Needs review

Addressed the mentioned comment , please review.

joachim’s picture

joachim commented
Status: Needs review » Reviewed & tested by the community

Thanks!

I think this is ready now.

kleiton_rodrigues’s picture

kleiton_rodrigues commented
StatusFileSize
new issues_3458295_18.png48.92 KB

@shalini_jha
RTBC++

  • quietone committed 44b62751 on 10.4.x 
    Issue #3458295 by pooja_sharma, shalini_jha, prem suthar, joachim,...

  • quietone committed b21bcf4e on 11.x 
    Issue #3458295 by pooja_sharma, shalini_jha, prem suthar, joachim,...

  • quietone committed bd53cc0b on 11.0.x 
    Issue #3458295 by pooja_sharma, shalini_jha, prem suthar, joachim,...

  • quietone committed 7486417d on 10.3.x 
    Issue #3458295 by pooja_sharma, shalini_jha, prem suthar, joachim,...
quietone’s picture

quietone commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 11.x, 11.0.x, 10.4x, and 10.3.x

Thanks!

edit: forgot to mention that this was not wrapped correctly and was fixed on commit.

Status: Fixed » Closed (fixed)

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