Follow up from #2066219-9: Decouple image type from image extension.

In https://drupal.org/node/608152#naming it states that methods declared in interfaces must be prefixed with public even if they are already public and PHP doesn't request explicitly to declare them with "public" prefix.

grep -nr "  function" core | grep Interface.php | wc -l
34
grep -nr "  static function" core | grep Interface.php | wc -l
1
CommentFileSizeAuthor
#9 public-in-interfaces-2066879-8.patch10.41 KBclaudiu.cristea
#8 typed-data-file-image-2015697-28.patch55.64 KBclaudiu.cristea
#2 public-in-interfaces-2066879-2.patch10.43 KBclaudiu.cristea

Comments

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue tags: +Novice

Tagging

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Active » Needs review
StatusFileSize
new public-in-interfaces-2066879-2.patch10.43 KB

Here's a patch.

Drupal\system\Plugin\ImageToolkitInterface is left out because is handled in #2066219: Decouple image type from image extension and I want to avoid conflicts.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

All of them seemed to be helpful as public functions.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Component: documentation » base system

Not documentation... I'm not going to touch this one!

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Component: base system » other

Well, this patch doesn't have any effect on coding. It cannot bring any error, bug or change to the way of how Drupal works. It's only about coding standards that's why I moved to documentation.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Ah. You can tag an issue "coding standards" to indicate it is about coding standards (which has already been done). But this still isn't documentation, and I am not sure about the assertion that this will not cause any problems, so I am going to let one of the more code-oriented/authorized committers handle this one.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

git ac https://drupal.org/files/public-in-interfaces-2066879-2.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 10676  100 10676    0     0  10438      0  0:00:01  0:00:01 --:--:-- 13180
error: patch failed: core/modules/editor/lib/Drupal/editor/Plugin/EditorPluginInterface.php:26
error: core/modules/editor/lib/Drupal/editor/Plugin/EditorPluginInterface.php: patch does not apply
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
Issue tags: -Novice, -Needs reroll
StatusFileSize
new typed-data-file-image-2015697-28.patch55.64 KB

Rerolled.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
StatusFileSize
new public-in-interfaces-2066879-8.patch10.41 KB

Sorry, attached a wrong patch :)

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Fine!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 13ee73e and pushed to 8.x. Thanks!

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