Problem/Motivation

\Drupal\auto_entitylabel\Form\AutoEntityLabelForm has:

  • Some unused use statements
  • Some minimally-documented properties
  • Some unnecessary @var comments
  • A @codingStandardsIgnoreLine directive
  • A docblock that refers to an older name for the class
  • A multi-line comment that does not conform to coding standards
  • A @TODO statement that suggests that a config factory is being used incorrectly.

Proposed resolution

Fix these issues.

Remaining tasks

  1. Submit a patch
  2. Review and feedback
  3. RTBC and feedback from maintainer
  4. Commit

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#8 2945361-8.patch7.97 KBshubham.prakash
#5 auto_entitylabel-2945361-5-cleanup-AutoEntityLabelForm.patch9.37 KBpancho
#3 interdiff.txt1.24 KBmparker17
#3 2945361-3-cleanup-AutoEntityLabelForm.patch8.56 KBmparker17
#2 2945361-2-cleanup-AutoEntityLabelForm.patch8.52 KBmparker17

Comments

mparker17 created an issue. See original summary.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Assigned: mparker17 » Unassigned
Issue summary: View changes
Status: Active » Needs work
StatusFileSize
new 2945361-2-cleanup-AutoEntityLabelForm.patch8.52 KB

Here is a patch that fixes the following issues reported in the original issue summary:

  • Some unused use statements
  • Some minimally-documented properties
  • Some unnecessary @var comments
  • A @codingStandardsIgnoreLine directive
  • A docblock that refers to an older name for the class
  • A multi-line comment that does not conform to coding standards

Still to do:

  • A @TODO statement that suggests that a config factory is being used incorrectly.
mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 2945361-3-cleanup-AutoEntityLabelForm.patch8.56 KB
new interdiff.txt1.24 KB

Turns out the last item was really easy because \Drupal\Core\Form\FormBase defines its own interface for accessing the configuration factory.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Status: Needs review » Postponed
Related issues: +#2923876: Broken when provider !=entityTypeId

Looking at #2923876: Broken when provider !=entityTypeId in depth; the changes made in this patch will conflict with the changes in that issue. Since the proposed changes in that issue fix broken functionality; while the changes in this issue do not, I feel it can be postponed until 2923876 is merged — once that happens, it is possible we can simply close this issue.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
StatusFileSize
new auto_entitylabel-2945361-5-cleanup-AutoEntityLabelForm.patch9.37 KB

Re-rolled against latest dev with #2923876-28: Broken when provider !=entityTypeId rolled in.

dww’s picture

dww
we/he/they
commented
Status: Postponed » Needs work
Issue tags: +Needs reroll

No longer applies to 8.x-2.x nor 8.x-3.x.
Interested in getting this in first to make #2945387: Support entities without bundles (User, etc.) easier to review / test.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

@dww I'd be happy to help, but I only have paid contribution time on Friday afternoons. I'll add it to my queue for then.

shubham.prakash’s picture

shubham.prakash commented
Version: 8.x-2.x-dev » 8.x-3.x-dev
Status: Needs work » Needs review
StatusFileSize
new 2945361-8.patch7.97 KB

Added reroll for latest dev branch..

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Issue tags: -Needs reroll

Removing the tag as the patch was rerolled.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje
Status: Needs review » Needs work
spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
spokje’s picture

spokje
Primary language Dutch
commented

Let's first get #3220398: Fix Coding Standard Messages in.