Problem/Motivation

This issue is to add documentation for the following source plugins:

  • d6/Node.php
  • d6/NodeComplete.php
  • d6/NodeRevision.php
  • d6/NodeType.php
  • d6/ViewMode.php
  • d7/Node.php
  • d7/NodeComplete.php
  • d7/NodeEntityTranslation.php
  • d7/NodeRevision.php
  • d7/NodeType.php
  • d6/Term.php
  • d6/TermLocalizedTranslation.php
  • d6/TermNode.php
  • d6/TermNodeRevision.php
  • d6/Vocabulary.php
  • d6/VocabularyPerType.php
  • d6/VocabularyTranslation.php
  • d7/Term.php
  • d7/TermEntityTranslation.php
  • d7/TermLocalizedTranslation.php
  • d7/TermTranslation.php
  • d7/Vocabulary.php
  • d7/VocabularyTranslation.php

Proposed resolution

Do it.

Remaining tasks

Review, commit.

Issue fork drupal-3209353

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

Matroskeen created an issue. See original summary.

Matroskeen opened merge request !571

matroskeen’s picture

matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
commented
Status: Active » Needs review

Ready for review

quietone’s picture

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

@Matroskeen, thanks for another one!

I applied the patch locally and read the changes in PhpStorm. It all looks great, I didn't find any nits. :-)

I do want to comment that some of the changes are only to the summary lines. I see that this is done so that all the summary lines use a consistent style and make it explicit that this is for a Drupal 6 or Drupal 7 source. I think it is worth adding that to the summary line and means the reader does not have to scan the Namespace to figure that out.

So, RTBC it is.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

matroskeen’s picture

matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
commented
Status: Reviewed & tested by the community » Needs review

There were some formatting changes, so it needs review again.

I tried to change a base branch from 9.2.x to 9.3.x and got a huge diff with "Not currently mergeable" error.
I've downloaded a patch from MR and applied it successfully on top of the latest 9.3.x.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs review » Needs work

@Matroskeen, Looks good, just one grammar error to fix.

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

nishantghetiya’s picture

nishantghetiya commented
Status: Needs work » Needs review

@quietone i have made changes according to your suggestion in #7 Please review.

quietone’s picture

quietone commented
Status: Needs review » Needs work

@nishantghetiya, thanks for working on this. The change is the correct text but it does not meet Drupal coding standards. The standards state that comments are to be wrapped at 80 columns. See Drupal API documentation standards (general), I think the 9th point.
Cheers

matroskeen’s picture

matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
commented
Status: Needs work » Needs review

Something weird happened to commit history, but a diff should be fine.

quietone’s picture

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

@Matroskeen, apologies, I forgot it was a long holiday weekend here.

I reviewed the MR in #7 and only found 1 item to fix. That has now been resolved. So off we go!

Thanks.

  • catch committed 1640eb2 on 9.3.x 
    Issue #3209353 by Matroskeen, nishantghetiya, quietone: Add...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

  • catch committed cd4c588 on 9.2.x 
    Issue #3209353 by Matroskeen, nishantghetiya, quietone: Add...

Matroskeen closed merge request !571

Status: Fixed » Closed (fixed)

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