Follow-up to #2111209: Rename FormInterface::getFormID() to FormInterface::getFormId()

We are renaming the function to get it consistent with our coding standards.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because the current naming is not consistent with our coding standards.
Disruption There is no disruption because php method names are case insensitive. It is only for our coding standards.

It is allowed because there is no disruption.

CommentFileSizeAuthor
#9 2113243-9.patch4.69 KBrpayanm
#8 2113243-8.patch160.4 KBrpayanm
#8 2113243-interdiff.txt578 bytesrpayanm
#6 2113243-6.patch4.12 KBrpayanm
#3 drupal_2113243_3.patch3.81 KBXano
#3 interdiff.txt372 bytesXano
#2 2113243-2.patch3.31 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Btw, does anyone agree with me that the thing should actually be called BaseFormInterface instead of BaseFormIdInterface? I think I'll open a follow-up for that.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Active » Needs review
FileSize
2113243-2.patch3.31 KB

PhpStorm++

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
FileSize
interdiff.txt372 bytes
drupal_2113243_3.patch3.81 KB

Looks good! I found one more occurrence.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Huh, PhpStorm--

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll
rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2113243-6.patch4.12 KB

Niet! PHPStorm++

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work

It all looks good to me.

You only missed one in the file core/modules/config_translation/src/Form/ConfigTranslationFormBase.php .

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Needs work » Needs review
Issue tags: +Novice
FileSize
2113243-interdiff.txt578 bytes
2113243-8.patch160.4 KB

@daffie thank you!

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
FileSize
2113243-9.patch4.69 KB

upss wrong patch

daffie’s picture

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

All instances of BaseFormIdInterface::getBaseFormID() are renamed to BaseFormInterface::getBaseFormId().
The patch looks good, so I give it RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2113243-9.patch, failed testing.

Status: Needs work » Needs review

tstoeckler queued 9: 2113243-9.patch for re-testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Title: Rename BaseFormIdInterface::getBaseFormID() to BaseFormInterface::getBaseFormId() » Rename BaseFormIdInterface::getBaseFormID() to BaseFormIdInterface::getBaseFormId()
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

So this issue says it is renaming BaseFormIdInterface::getBaseFormID() to BaseFormInterface::getBaseFormId() - it does not actually rename the interface - only the method - well it fixes the capitalisation. I don't think that BaseFormIdInterface should become BaseFormInterface since FormBase and ConfigFormBase do not implement it - the interface is solely about provided a base ID - so I think it well named.

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

daffie’s picture

daffie CreditAttribution: daffie commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

Back to RTBC, because the patch does what the new title is saying and the beta phase evaluation is added.

@alexpott: Sorry for missing the renaming of the interface in the title. :-(

daffie’s picture

daffie CreditAttribution: daffie commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs issue summary update

Committed 2e6efcb and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.

  • alexpott committed 2e6efcb on 8.0.x 
    Issue #2113243 by rpayanm, Xano, tstoeckler: Rename BaseFormIdInterface...

Status: Fixed » Closed (fixed)

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