[commits] [Wiki] changed: Doc/Dev/ConversionH6
Ralf Lang (B1 Systems GmbH)
lang at b1-systems.de
Fri Nov 19 16:14:56 UTC 2021
rlang Fri, 19 Nov 2021 16:14:56 +0000
Modified page: https://wiki.horde.org/Doc/Dev/ConversionH6
New Revision: 6
Change log: Add table of conversion status
@@ -3,33 +3,78 @@
These are by far not complete.
+
+
+++ Library strategies
+
++++ Parallel
+Parallel strategy means double maint. burden but high freedom for due
improvements and necessary BC breaks.
+
+Example: horde/test
+
+* Keep unnamespaced PSR-0 code as-is or only make changes required by
composer.
+* Add PSR-4 PHP 7.4+ code in src/ - it may contain BC breaks and
structural changes.
+* develop new code quality measures only against the src/ version
(phpstan, member/parameter/return type hinting, strict types)
+* Convert unit tests for phpunit 9.x, use PSR-4 for unit tests, test
against the namespaced version
+* src/ Code must be php 7.4 compatible and php 8 compatible.
+
++++ Wrapper
+Wrapper strategy allows both old and new calling code to leverage the
same slightly modernized code base. However, BC breaks must be
prevented.
+
+Example: horde/components, horde/injector
+* Move code to src/ and namespace it
+* Add wrappers or child classes to lib so consuming code still works
+* Be conservative about return type hinting, parameter type hinting
and other potentially BC breaking measures
+* Convert unit tests for phpunit 9.x, use PSR-4 for unit tests, test
against the namespaced version - maybe keep some duplicate tests
against the wrapper code to prove correctness.
+* All Code must be php 7.4 compatible and php 8 compatible.
+
++++ Modern
+
+Newly developed code should only focus on the composer use case
+
+Example: horde/horde-installer-plugin, horde/http_server
+* Follow PSR-4, PSR-12, PHP 7.4+, ignore PEARisms unless they belong
to the problem domain.
+* PHPStan level 4 and higher.
+* member/parameter/return type hinting, strict types
+* Avoid mixed parameter and return type designs.
+* Avoid array constructors if it makes sense. (When switching to PHP
8, we will get named arguments)
+* src/ Code must be php 7.4 compatible and php 8 compatible.
+
++++ Keep
+
+Libraries which are essentially deprecated but kept around for H6
+
+Example: horde/controller
+
+* Only necessary changes to avoid breaking in modern PHP
+* unit tests should be PHPUnit 9 ready
++ General
* Unnamespaced code mostly follows Horde 5 Coding standards and very
little conversion requirements
-* Most of the upgrade requirements are only targeted at namespaced code
* With some necessary exceptions, Horde 6 should be able to
cooperate with Horde 5-ish apps and libraries with very little
modification for composer compat
-* Deprecate PSR-0 autoloading but don't break it intentionally.
Unconditional require/require_once/include to paths outside the
library's own lib or src dir need to check for the require'd class
first (likely breaks in H6 via composer).
+* Unconditional require/require_once/include to paths outside the
library's own lib or src dir need to check for the require'd class
first (likely breaks in H6 via composer).
[Unnamespaced code still needs Horde_Autoloader's slightly different
approach than composer's autoloader]
* Consider: Horde 5 Framework lived out 2012-2021, I would like
Horde 6 to be intentionally shortlived, deprecating more stuff over
time.
* Allow a very smooth transition path, avoid big bang changes. [STILL TRUE]
-
+
+++ Exceptions to the rule
* Most of the upgrade requirements are only targeted at namespaced code
-+++ Mandatory
-
++++ Mandatory
+
* Horde:: is deprecated [NO REAL REPLACEMENT YET]
* [rla thinks] we should adopt PSR-2, PSR-12 wholesale with an
optional opt-out for protected/private underscoring [RLA DOES THIS
ALREADY]
* Composer based setup must work with all libraries we intend to
keep [SOLVED]
* [rla thinks] Deprecate Horde_Forms usage. Keep it where it is
until we can refactor, but make it clear this is a legacy concept
- * Looked into it. There's a few parts to easily refactor when
converting to namespaced, with some BC breaks but very schematic
remedy. This will reduce warnings and improve compat with newer PHPs.
It will really make Form more attractive.
+ * Looked into it. There's a few parts to easily refactor when
converting to namespaced, with some BC breaks but very schematic
remedy. This will reduce warnings and improve compat with newer PHPs.
It will really make Form more attractive. Horde_Forms is a major deal
breaker for PHP 8.
+
* Update Horde_Core to support both namespaced apps non-namespaced
apps and mixed. [ONGOING]
-* Update Horde_Test to support namespaced unit tests (PHPUnit 9 spews
warnings for unit test classnames different than file name, so there
is little use in BC here)
-* Unit Tests must support the current stable phpunit [WIP]
+* Update Horde_Test to support namespaced unit tests (PHPUnit 9 spews
warnings for unit test classnames different than file name, so there
is little use in BC here) [DONE]
+* Unit Tests must support the current stable phpunit [DONE]
* Check all pear package dependencies if they are also available via
packagist or a proper composer channel rather than pear [MOSTLY DONE]
* Update skeleton to use
- * Horde Controllers,
+ * horde/http_server,
* Namespacing,
* Type Hints where appropriate and
* Model Interface and Rdo implementation rather than driver
architecture [NOT YET DONE]
@@ -41,8 +86,9 @@
* Unbundle bundled code if you can, use composer dependencies
[UNBUNDLED phpunit, sabredav]
* Maintain a branch or fork of Skeleton to showcase Horde Ajax
Framework and an SPA frontend
* All URL generation should be moved to Horde_Routes_UrlWriter
driven by config/routes.php [NOT DONE]
* A more copy/paste ready boilerplate for ClassLevelDocBlock,
FileLevelDocBlock, MethodLevelDocBlock
+ * Consider: Maybe some custom rules for php-cs-fixer
+++ Exceptions To The Rule
* ?
@@ -57,8 +103,9 @@
* TODO: Should we allow third party apps to use their proper
vendor if the composer package is type horde-app? We could have an
optional registry key (B1 clause)
* The Horde Base app is Horde\Base
* DISCUSS: Any arguments for calling it Horde\Horde? Should we
rename/alias the base app horde/base altogether?
+ * Base is an even worse name than "horde". Between Base and Core,
things are often mixed or simply on the wrong side.
* If a library has an interface or base class Horde_$Name, promote
it to either:
- Horde\$Name\$Name
- Horde\$Name\$NameInterface
@@ -67,32 +114,33 @@
- In case of base classes, re-think if we are not better off with
an interface and some trait
* Horde:: is Horde\Core\Horde:: though I think we should re-think
using it at all
-* Un-namespaced controllers go to App\, namespaced controllers go to
src/Controller
+* Un-namespaced controllers go to app\controllers\ dir, namespaced
controllers just go anywhere under src/, i.e. src/Middleware,
src/Handler, src/Controller with no magic attached.
* Un-namespaced library code goes to lib/
* New or converted namespaced library code goes to src/.
+++ Optional
-* Unnamespaced code should be preceded with backslash \ to ensure we
do not run into issues with namespaced derived classes or use cases
- * Do it where it prevents breaks or where . Don't waste effort.
+* Prefer Use statements over prepending unnamespaced code with a
backslash \ unless it is single-use and would require "use as" to
avoid clashes.
+
+++ Exceptions to the rule
-
-* Code which was already namespaced before Horde 6 may be left in
lib/ if other unnamespaced code depends on it
-
-
-
-++ PHPDoc / Type Hints / Type Declarations
+
+* Code which was already namespaced before Horde 6 may be left in
lib/ if other unnamespaced code depends on it.
+ * In this case, don't forget to tweak autoloading rules in
.horde.yml and adjust any github actions workflows
+
+
+
+++ PHPDoc / Type Hints / Type Declarations
Remember Liskov:
* Interface/Base class parameters should be as specific as possible.
Derived/Implementing classes can only accept the same or less specific
definitions
* Interface/Base class return type parameters should be as
unspecific as makes sense. Derived/Implementing classes can announce
to return the same or more specific definitions.
-
-+++ Mandatory
-
+
++++ Mandatory
+
* PHPDoc all parameters and return values, regardless if you have
typehinted them.
* Prefer A|B|null over "mixed"
+++ Optional
@@ -143,34 +191,34 @@
+++ Exceptions to the rule
* ?
-
+
++ DI / Injector and Constructors
-
+
As Horde has evolved over the years, we have different types of
classes, sometimes even mixed.
* Classes which implement create-on-the-fly objects with no/few
dependencies or are mostly static, e.g. Horde_Url, Horde_Date,
Horde_String
* Classes which have one or few instances to cover most or all use
cases, e.g. the DB Driver, the registry. They will typically be
provided by the injector directly or via a factory without explicit
parameters
* Messy stuff which cannot decide if it is a factory or a base class
or wants composition, e.g. Horde_Form, Horde_Rpc
* Classes which are intended as Per-Entity objects
+++ Mandatory
-
+
* Constructors SHOULD depend on interfaces or base classes, not on
the (single) implementation.
* HordeBase or App configuration SHOULD be depended on as *TODO*
object, not as array and explicitly not via global state
* Apps must not care for configuration of other apps, only theirs or
HordeBase
* Namespaced app-internal objects which implement an interface
should be registered with the injector using the class combined with
::class, e.g.
-
+
<code type="php">
// Use a factory to get an implementation
// In case the factory cannot be autowired itself, you need
to register how to get it first.
$injector->bindFactory(Horde\Kronolith\Calendar\Resource::class,
Horde\Kronolith\Calendar\Factory::class, 'create')
// In case you want a specific implementation and it can be
autowired, just bind it.
$injector->bindImplementation(Horde\Kronolith\Renderer::class,
Horde\Kronolith\Renderer\Default::class);
-</code>
-
+</code>
+
* Note there are no strings, this is the class itself - ::class
returns the fully qualified strings
* If you have imported the class with use, you can just use the
imported name:
<code type="php">
@@ -220,10 +268,47 @@
+++ UNCLEAR
* Do we still want to support git-tools setups?
* I have implemented some basic git handling in components now.
+
+
++ Conversion Status
+
+||~ Package ||~ Strategy ||~ Unit Tests ||~ PHPStan ||~ PHP 8 ||~
Comments/Upgrade path ||
+|| horde/components || Wrapper || 9.4, Story tests broken || ? || ?
|| Lots of new features related to git, composer and workflows. Still
maintains package.xml and requires it in some places ||
+|| horde/horde-installer-plugin|| Modern || 9.4, very limited ||
Level 8 || Yes || Handles all the horde setup stuff, symlinks,
workaround configs etc ||
+|| horde/horde-deployment|| Modern || n/a || n/a || Yes || The base
project for a horde installation with default dir tree - Branches
reflect bundles ||
+|| horde/http_server || Modern || 9.4 || Level 2 || yes || Successor
to horde/controller ||
+|| horde/http || Parallel || 9.4 || Level 2 || yes || Major BC
breaks. Implementation based on PSR-7, PSR-18 ||
+|| horde/injector || Wrapper || 9.4 || Level 2 || ? || Carefully
added signatures, limited BC breaks for child injectors. Now PSR-11 ||
+|| horde/controller || Keep || 9.4 || ? || ? || Only added
namespaced wrapper code for interop with horde/http_server and PSR-7 ||
+|| horde/stream_wrapper || Parallel || 9.4 || Level 8 || yes || Class
names have Wrapper appended to make sense when USEd ||
+|| horde/util || Parallel || 9.4 || Level 0 || yes || Slightly
renamed classes and typing upgrades ||
+|| horde/support|| Parallel || 9.4 || Level 1 || yes || Slightly
renamed classes and typing upgrades ||
+|| horde/compress_fast || Parallel || 9.4 || Level 1 fails due to
reliance on horde-specific extension || yes || Straight ||
+|| horde/mongo || Parallel || 9.4 || Level 1 || yes || Removed
support for the older extension - Still using the compat library even
though it makes little sense by now ||
+|| horde/cli || Parallel || 9.4 || Level 1 fails || yes || straight ||
+|| horde/memcache || Parallel || 9.4, started minimal test suite ||
Level 4 || yes || straight ||
+|| horde/cache || Parallel || 9.4, started minimal test suite ||
Level 1 || yes || Dropped some dead in-memory caches ||
+|| horde/test || Parallel || 9.4, started minimal test suite || Level
1 || yes || straight ||
+|| horde/routes || Parallel || 9.4 || Level 1 || yes || straight ||
+
+
+
+
+
+
+
+
+
+
+
+
+ package specific notes
+
+
+
++ Horde\Injector
+++ Optional
* Implement PSR-11 but do not deprecate getInstance(), setInstance()
[PR exists]
More information about the commits
mailing list