Problem/Motivation

After merge of #2984803: Transform PHP Native class from Plugin Factory we have some standards and documentation miss in code base.

Proposed resolution

Fix errors and refactor/document all obscures methods.

CommentFileSizeAuthor
#6 2985368-optimise-plugin-factory-6.patch32.1 KBwoprrr
#4 2985368-optimise-plugin-factory-4.patch20.71 KBwoprrr
#2 2985368-optimise-plugin-factory.patch20.59 KBwoprrr

Comments

woprrr created an issue. See original summary.

woprrr’s picture

woprrr commented
Status: Active » Needs review
StatusFileSize
new 2985368-optimise-plugin-factory.patch20.59 KB
woprrr’s picture

woprrr commented
Status: Needs review » Needs work 
+++ b/src/AbstractAutomatedCrop.php
@@ -211,24 +220,13 @@ abstract class AbstractAutomatedCrop extends PluginBase implements AutomatedCrop
+  public function setAnchor(array $coordinates = []) {
     return array_merge($coordinates, $this->cropBox);
   }

Best practice miss !! this not respect the return in Interface this will break LSP. We should return $this here.

woprrr’s picture

woprrr commented
Status: Needs work » Needs review
StatusFileSize
new 2985368-optimise-plugin-factory-4.patch20.71 KB

Here the patch with LSP violation fixed.

woprrr’s picture

woprrr commented
Status: Needs review » Needs work

We have a last major issue to be really SOLID :)

see https://scrutinizer-ci.com/g/woprrr/automated_crop/code-structure/8.x-1....

The code duplication in AutomatedCropDefault plugins need to be fixed.

woprrr’s picture

woprrr commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 2985368-optimise-plugin-factory-6.patch32.1 KB

Woot ! woot ! https://scrutinizer-ci.com/g/woprrr/automated_crop/inspections/490aa170-...

9.99 :D not bad.

  • woprrr committed edf38dc on 8.x-1.x 
    Issue #2985368 by woprrr: Optimise Plugin Factory
woprrr’s picture

woprrr commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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