ImageFieldTestBase::createImageField() accepts a $field_settings parameter that is supposed to contain both the settings, and the 'description' for the field, next to each other in the same set of array keys.
That makes no sense, and would clash if there was a setting called 'description'.
Not sure that helper method even needs to bother with setting a field description ?
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | remove-description.patch | 2.39 KB | gábor hojtsy |
| #2 | remove-description.patch | 2.39 KB | gábor hojtsy |
| #2 | move-description.patch | 2.88 KB | gábor hojtsy |
Comments
Comment #1
yched commentedGood novice task :-)
Comment #2
gábor hojtsyWanted to make sure to attend to this followup, so even though marked as Novice, here it is :) One version to move the argument (but only found one use). And one version to remove it (which as you said should hopefully be even better).
Comment #3
tibbsa commentedAt the moment, there is no testing purpose for keeping the description, I don't believe. But for the sake of future extensibility, making it apparent that this is a "possibility" is probably not a bad thing. I'd vote for "moving" rather than "removing".
Comment #4
yched commentedThanks @Gabor :-)
No big deal, but I'd vote for "remove". There's little point in keeping the description IMO.
If a specific test needs to specifically test something with the description (should be rare, the "description" is UI-only...), it can always set it itself. This is only a helper method for brevity, it's only normal that it doesn't have all the verbosity and features of calling the actual "create" functions itself.
+ FileFieldTestBase has a similar createFileField() that doesn't take a description, so... consistency++ :-)
So, RTBC for me for the "remove patch".
Comment #5
gábor hojtsyReuploading for easier committer review then.
Comment #6
yched commented:-)
Comment #7
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 49b1c1d and pushed to 8.0.x. Thanks!