According to OOP best practice and the coding standards image.module should be hinting with an interface instead of with a class. (I don't know if there is a meta issue for this already)

Change the use statement at the top from

use Drupal\file\Entity\File;

to

use Drupal\file\FileInterface;

and change all the type hints from

File

to

FileInterface

CommentFileSizeAuthor
#13 image.module-should-use-FileInterface-instead-of-File-2897979-13.patch1.13 KBsvetoslav.dragoev
#10 interdiff-2897979-8-10.txt554 byteskarthikkumarbodu
#10 image_module_should_use_FileInterface_instead_of_File-2897979-10.patch123.22 KBkarthikkumarbodu
#8 interdiff-2897979-6-8.txt1.35 KBkarthikkumarbodu
#8 image_module_should_use_FileInterface_instead_of_File-2897979-8.patch123.25 KBkarthikkumarbodu
#6 interdiff-2897979-4-6.txt2.47 KBkarthikkumarbodu
#6 image_module_should_use_FileInterface_instead_of_File-2897979-6.patch123.34 KBkarthikkumarbodu
#4 image_module_should_use_FileInterface_instead_of_File-2897979-4.patch123.53 KBkarthikkumarbodu

Comments

vegantriathlete created an issue. See original summary.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

michaellenahan’s picture

michaellenahan commented
Issue tags: +Novice, +Vienna2017
karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Active » Needs review
StatusFileSize
new image_module_should_use_FileInterface_instead_of_File-2897979-4.patch123.53 KB

Status: Needs review » Needs work

The last submitted patch, 4: image_module_should_use_FileInterface_instead_of_File-2897979-4.patch, failed testing. View results

karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Needs work » Needs review
StatusFileSize
new image_module_should_use_FileInterface_instead_of_File-2897979-6.patch123.34 KB
new interdiff-2897979-4-6.txt2.47 KB

Status: Needs review » Needs work

The last submitted patch, 6: image_module_should_use_FileInterface_instead_of_File-2897979-6.patch, failed testing. View results

karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Needs work » Needs review
StatusFileSize
new image_module_should_use_FileInterface_instead_of_File-2897979-8.patch123.25 KB
new interdiff-2897979-6-8.txt1.35 KB

Status: Needs review » Needs work

The last submitted patch, 8: image_module_should_use_FileInterface_instead_of_File-2897979-8.patch, failed testing. View results

karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Needs work » Needs review
StatusFileSize
new image_module_should_use_FileInterface_instead_of_File-2897979-10.patch123.22 KB
new interdiff-2897979-8-10.txt554 bytes

Status: Needs review » Needs work

The last submitted patch, 10: image_module_should_use_FileInterface_instead_of_File-2897979-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

svetoslav.dragoev’s picture

svetoslav.dragoev commented

Counted from today, Me (Svetoslav Dragoev), Nevena Kostova and Yasen Ivanov would be taking up the task. God speed all!

svetoslav.dragoev’s picture

svetoslav.dragoev commented
Status: Needs work » Needs review
StatusFileSize
new image.module-should-use-FileInterface-instead-of-File-2897979-13.patch1.13 KB

We've provided the patch over the image.module.

somepal’s picture

somepal commented

reviewed

somepal’s picture

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

  • larowlan committed d361241 on 8.5.x 
    Issue #2897979 by karthikkumarbodu, svetoslav.dragoev: image.module...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

used grep -Er " ?File(;| |\(|:)\$?" core/modules/image/ to find any remaining references that needed update - found none.

Committed as d361241 and pushed to 8.5.x

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +DrupalSouth 2017

Status: Fixed » Closed (fixed)

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