Followup after #2143263: Remove "Field" prefix from FieldDefinitionInterface methods, and postponed on it.

WidgetBase & FormatterBase also have getSetting[s]() methods to "access own settings", this would be consistent.

CommentFileSizeAuthor
#28 get-field-setting-left-over-2152825-28.patch851 bytesberdir
#25 remove_field_prefix-2152825-25.patch22.66 KBswentel
#24 remove_field_prefix-2152825-24.patch22.65 KBberdir
#22 remove_field_prefix-2152825-22.patch22.65 KBplopesc
#22 intediff.txt1.13 KBplopesc
#17 remove_field_prefix-2152825-17.patch21.04 KBplopesc
#9 remove_field_prefix-2152825-9.patch21.09 KBplopesc
#4 remove_field_prefix-2152825-4.patch16.82 KBplopesc

Comments

yched’s picture

yched commented

Also, we might consider adding similar methods at the level of FieldItemList ?

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented

Should FieldItemBase::getFieldDefiniton also renamed to getDefinition()?

amateescu’s picture

amateescu commented
Status: Postponed » Active

Should FieldItemBase::getFieldDefiniton also renamed to getDefinition()?

Not sure about this, since we have a FieldDefinitionInterface I think it makes sense to "getFieldDefiniton"..

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new remove_field_prefix-2152825-4.patch16.82 KB

First approach, only renaming methods described in issue summary. Waiting for definitive decission about #1.

Regards.

yched’s picture

yched commented

4: remove_field_prefix-2152825-4.patch queued for re-testing.

yched’s picture

yched commented

re #1, "adding getFieldSetting[s]() methods on FieldItemList" - yes, I'd think we want to do this...

yched’s picture

yched commented
Issue tags: +Entrity Field API

tagging

Status: Needs review » Needs work

The last submitted patch, 4: remove_field_prefix-2152825-4.patch, failed testing.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new remove_field_prefix-2152825-9.patch21.09 KB

Re-rolling and addressing suggestion in #1.

Regards.

yched’s picture

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

Looks good. Thanks @plopesc !

yched’s picture

yched commented

9: remove_field_prefix-2152825-9.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: remove_field_prefix-2152825-9.patch, failed testing.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review

9: remove_field_prefix-2152825-9.patch queued for re-testing.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC, testbot random fails...

xjm’s picture

xjm
she/her
Primary language English
commented

9: remove_field_prefix-2152825-9.patch queued for re-testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Last test on this failed but never got reported back https://qa.drupal.org/pifr/test/689243

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
Issue tags: -Entrity Field API +Entity Field API
StatusFileSize
new remove_field_prefix-2152825-17.patch21.04 KB

Straight re-roll

yched’s picture

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

Thanks @plopesc

The last submitted patch, 9: remove_field_prefix-2152825-9.patch, failed testing.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented

17: remove_field_prefix-2152825-17.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: remove_field_prefix-2152825-17.patch, failed testing.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new intediff.txt1.13 KB
new remove_field_prefix-2152825-22.patch22.65 KB

Re-rolling after #2015689: Convert field type to typed data plugin for options module

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Will cause commit conflicts

No longer applies, sorry.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new remove_field_prefix-2152825-24.patch22.65 KB

Re-roll.

swentel’s picture

swentel commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new remove_field_prefix-2152825-25.patch22.66 KB

rerolled

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +API change

Looks like https://drupal.org/node/1805846 will need an update when this goes in.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Fixed one line in the change notice that needed updating, I think that's it so moving direct to fixed.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: rename FieldItemBase::getFieldSetting[s]() to getSetting[s]() » HEAD BROKEN: rename FieldItemBase::getFieldSetting[s]() to getSetting[s]()
Category: Task » Bug report
Priority: Normal » Critical
Status: Fixed » Active
Issue tags: -Will cause commit conflicts
StatusFileSize
new get-field-setting-left-over-2152825-28.patch851 bytes

Uh oh, #2002168: Convert form validation of terms to entity validation introduced a new call to getFieldSetting() that we missed here, HEAD is going to fail due to this.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: HEAD BROKEN: rename FieldItemBase::getFieldSetting[s]() to getSetting[s]() » rename FieldItemBase::getFieldSetting[s]() to getSetting[s]()
Priority: Critical » Normal
Status: Active » Fixed 
Drupal test run
---------------

Tests to be run:
 - Add feed functionality (Drupal\aggregator\Tests\AddFeedTest)

Test run started:
 Thursday, February 13, 2014 - 14:44

Test summary
------------

Add feed functionality 67 passes, 0 fails, and 0 exceptions

Yep this fixes the HEAD errors.

Committed 60d7878 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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