[dev] [commits] Horde branch master updated. a90e4c02a538981f772a45205c991d466f9d472a

[Jan Schneider]

Zitat von Ralf Lang <lang at b1-systems.de>:

> The branch "master" has been updated.
> The following is a summary of the commits.
>
> from: ccafece43576bd2fb5c0c2fe63b7432220daf80c
>
> a90e4c0 Implement modular cli and a config editing module (Request #12923).
>
> Summary:  
> http://github.com/horde/horde/compare/ccafece43576bd2fb5c0c2fe63b7432220daf80c...a90e4c02a538981f772a45205c991d466f9d472a
>
> -----------------------------------------------------------------------
>
> commit a90e4c02a538981f772a45205c991d466f9d472a
> Author: Ralf Lang <lang at b1-systems.de>
> Date:   Thu Jan 23 21:44:46 2014 +0200
>
>     Implement modular cli and a config editing module (Request #12923).
>
>  framework/Core/lib/Horde/Config.php  |   29 +++++-
>  framework/Core/package.xml           |    2 +
>  horde/bin/horde-cli                  |   29 +++++
>  horde/docs/CHANGES                   |    1 +
>  horde/lib/AdminCli.php               |   77 ++++++++++++
>  horde/lib/AdminCli/Module/Base.php   |   94 ++++++++++++++
>  horde/lib/AdminCli/Module/Config.php |  221  
> ++++++++++++++++++++++++++++++++++
>  horde/package.xml                    |  221  
> +++++++++++++++++++++++++++++++++-
>  8 files changed, 672 insertions(+), 2 deletions(-)
>  create mode 100755 horde/bin/horde-cli
>  create mode 100644 horde/lib/AdminCli.php
>  create mode 100644 horde/lib/AdminCli/Module/Base.php
>  create mode 100644 horde/lib/AdminCli/Module/Config.php
>
> http://github.com/horde/horde/commit/a90e4c02a538981f772a45205c991d466f9d472a
> http://git.horde.org/horde-git/-/commit/a90e4c02a538981f772a45205c991d466f9d472a

There are several issues with this commit:
- horde-cli is a much too generic name. horde-config, horde-configure,  
horde-configuration, horde-admin are much more suitable names.
- This library doesn't belong into horde, but into Horde_Core
- The class name is completely wrong, it must be Horde_AdminCli or  
Horde_Core_AdminCli
- Please fix the coding style of the page/class header docs.
- There are a few incorrect indentions.
- The module base class doesn't provide any basic functionality, so it  
doesn't make sense. OTOH the only actual module class doesn't  
implement Horde_Cli_Modular_Module.
- You have included in package.xml that don't have anything to do with  
the patch.
- The XML tree of conf.xml already exists in Horde_Config#_xmlConfigTree.

I suggest you revert the patch and prepare a PR instead that we can  
discuss further before being merged.
-- 
Jan Schneider
The Horde Project
http://www.horde.org/
https://www.facebook.com/hordeproject