Problem/Motivation

The properties in MigrateException have doc comments like this:

  /**
   * The level of the error being reported.
   *
   * The value is a Migration::MESSAGE_* constant.
   *
   * @var int
   */
  protected $level;

  /**
   * The status to record in the map table for the current item.
   *
   * The value is a MigrateMap::STATUS_* constant.
   *
   * @var int
   */
  protected $status;

The problem is that in an IDE or on api.d.o, those don't form links.

Each of these should have a @see line added to MigrateIdMapInterface and MigrationInterface respectively.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3250683

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

beatrizrodrigues’s picture

Assigned: Unassigned » beatrizrodrigues

I'll work on that :)

joachim’s picture

Ok great!
Note that I was lazy and I didn't look up the full class names of MigrateIdMapInterface and MigrationInterface -- in the @see they need to be the namespaced class name.

beatrizrodrigues’s picture

Assigned: beatrizrodrigues » Unassigned
Status: Active » Needs review

thanks @joachim I looked up the namespaces, but thank you for the reminder. I did the MR, I hope everything is right. Also, in the issue's description is saying that

Each of these should have a @see line added to MigrateIdMapInterface and MigrationInterface respectively.

So, I thought that the $level might have the MigrationInterface @see line, and the other, the MigrateIdMapInterface @see line. But I saw in the construct method, that was the opposite of that. I don't know if a got the description wrong or it was inverted indeed.

beatrizrodrigues’s picture

Sorry for the mess, I did the previous MR on the wrong version.

joachim’s picture

Status: Needs review » Reviewed & tested by the community

> I don't know if a got the description wrong or it was inverted indeed

I got the order wrong in the issue summary, sorry!

RTBC -- the MR looks perfect. Thanks!

quietone’s picture

Status: Reviewed & tested by the community » Needs work

With the suggested change the doc block has two different paths for the constants. I considered removing the sentences that start 'The value is a ...' but decided to keep them because they identify which constants are used. So, lets change those lines to point to the interface as well.

ravi.shankar made their first commit to this issue’s fork.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Tauany Bueno’s picture

Assigned: Unassigned » Tauany Bueno
Status: Needs work » Needs review

hey, i'll review it :)

Tauany Bueno’s picture

Assigned: Tauany Bueno » Unassigned

Hello!
I worked on the issue and fixed the problem mentioned on comment #8. Also, the MR from comment #9 removed some changes made by Beatriz that were correct.
I'm changing the status to needs review :)

ps: it needs to be rebased, however I saw that the option to rebase is available on GitLab, so I didn't work on this.

Tauany Bueno’s picture

I committed the wrong changes for comment #8. Made a new push to fix it :)
sorry for the mess

Joel Guerreiro Borghi Filho’s picture

Assigned: Unassigned » Joel Guerreiro Borghi Filho

Hi! Will review =).

Joel Guerreiro Borghi Filho’s picture

Assigned: Joel Guerreiro Borghi Filho » Unassigned
Status: Needs review » Reviewed & tested by the community

After reviewing the code and talking to @tauanygb, I checked the latest changes, and they are looking good to me. Changing status to RTBC :)

quietone’s picture

@Joel Guerreiro Borghi Filho, thanks for commenting on what you did to decide this was RTBC.

I resolved the threads in the MR.

I should be able commit this. I'll wait 48 hours for another committer to respond.

alexpott’s picture

Version: 9.5.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 88dceab07b to 10.1.x and be7f60d445 to 10.0.x and ae4cc2e6b3 to 9.5.x and 1ee191b795 to 9.4.x. Thanks!

Backported to 9.4.x since this is a docs improvement.

  • alexpott committed 88dceab on 10.1.x
    Issue #3250683 by beatrizrodrigues, Tauany Bueno, ravi.shankar, joachim...

  • alexpott committed be7f60d on 10.0.x
    Issue #3250683 by beatrizrodrigues, Tauany Bueno, ravi.shankar, joachim...

  • alexpott committed ae4cc2e on 9.5.x
    Issue #3250683 by beatrizrodrigues, Tauany Bueno, ravi.shankar, joachim...

  • alexpott committed 1ee191b on 9.4.x
    Issue #3250683 by beatrizrodrigues, Tauany Bueno, ravi.shankar, joachim...

Status: Fixed » Closed (fixed)

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