Problem/Motivation

I think in general we should move to reducing the amount of procedural code so more effective tests can be written. It would also be good to write tests for the key components of the module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152 created an issue. See original summary.

Sam152’s picture

Since there are a lot of dependencies between the sub-issues in this ticket, they are all on one branch. Attached is the whole branch. It could become tricky juggling feedback between dependent patches, but we'll see.

Status: Needs review » Needs work

The last submitted patch, 2: 2642154-reduce-procedural-code.patch, failed testing.

Sam152’s picture

Status: Needs work » Needs review
frjo’s picture

Thanks for helping to improve and modernise the Colorbox module. I have committed you patches and it seems to work. Now I just need to sit down and understand how it works.

I have not really had time to sit down and understand all this object oriented / Symfony stuff in D8. For me at the moment it just seems to add a lot of complexity. The D8 version now have more than double the number of code files from the D7 version, 20+ and counting.

Sam152’s picture

Yeah, Drupal 8 is definitely way more heavy with how many files are required and some of the APIs are a lot more verbose. I do believe it will lead to better code in the long term however.

Sam152’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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