META - improve code quality
I'm writing this as a place to group together various efforts to improve our code quality - this means we can link towards other issues that reflect where we are going code-wise.
In general from a product maintenance point of view the goals we have are
- ongoing improvement in code stability through
- adding test coverage,
- fixing things 'the right way' rather than hack-fixes
- addressing buggy coding
- improving code commenting
- addressing underlying logic problems
- cleaning up known bad patterns
- extracting smaller functions out of larger functions
- improving code readability
- not adding new functionality (fields, logic etc and even bug fixes) to dirty code (either cleaning it up first or leaving well enough alone).
Note that is IS harder to get changes accepted into parts of the code which are nasty - this is a feature not a bug. It's also harder to get changes reviewed & accepted into areas with poor test coverage. As I write this prs to alter sending out invoices and code impacting on imports are notably stagnating. This reflects an unwillingness to touch those code areas unless unit tests are being added as they have almost no coverage. (of course we also want tests on more tested code areas...)
- ongoing improvement in performance through
- removing bad joins & bad queries
- php improvements
-
ensuring secure code practices are used
-
adapting the code to new versions of mysql & php as they become relevant
-
laying the ground work for LeXIM through
- moving logic out of the form layer and into the BAO, api xml or other metadata where appropriate
- improving api interaction with the code (making more stuff api-accessible & making it more logical & consistent)
- adding hooks, regions etc, although not into dirty code as that locks in suboptimal fixes.
- improving hook consistency
There are other goals for CiviCRM as a product - in the Leap By Extension / funded projects part of the LeXIM picture which are outside the scope of the concerns of product maintenance. It should be noted that the there is limited capacity to review PRs submitted to core so fit with these will affect how high a priority product mtce reviewers give to PRs. Of course other things play into the prioritisation like whether the submitter has been actively reviewing other PRs.
I'm going to start adding / linking issues that relate to cleaning up known bad patterns here
Related issues
Remove ->free calls #562 (closed)
Performance - done, remove use of mysql LOWER
Switch core forms to Entity Forms #818 (closed) - also #115 (closed)
Get rid of jcalendar #561 (closed)
Use TempTable class for all temp tables - #183 (closed)
Consolidate settings & preference forms onto being metadata driven #495 (closed)
Consolidate recurring forms #846 (closed)
Use guzzle to get http requests #849 (closed) (for testability)
Get rid of nullArray usage #1047 (closed)
Remove non-std contribution_invoice_settings #1558 (closed)