[Koha-bugs] [Bug 7804] Add Koha Plugin System

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Thu Jan 10 13:31:43 CET 2013


http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7804

--- Comment #49 from Jonathan Druart <jonathan.druart at biblibre.com> ---
Kyle,

Great Job! I really like the idea behind this patch and this new rewrite is
more elegant.

Just 2 short remarks after my first tests:

- Module::Bundled::Files is required (installed via cpan) and is not in the
perl deps
- The enable_plugins entry in the koha-conf file is not in the test plan

Some little stuffs (but this feature is in discussion so I don't know if it is
relevant):
- 8 spaces in tool-plugins.tt
- the ERROR_NO_TOOLS in never set
- $version in Plugins.pm is never used
- Plugins.pm:
            if ( defined($method) && $plugin->can($method) ) {
                push( @plugins, $plugin );
            } else {
                push( @plugins, $plugin );
            }

the tools/tool-plugins is completely broken (TOOLS_LOOP in tt vs plugins in
pl), I suspect you did not commit them on purpose.

qa tools complains on 3 small fails:
 * Koha/Plugins.pm                                                         
FAIL
  pod                         FAIL
    *** WARNING: Verbatim paragraph in NAME section  in file Koha/Plugins.pm
    *** ERROR: =item without previous =over  in file Koha/Plugins.pm
 * Koha/Plugins/Base.pm                                                    
FAIL
  pod                         FAIL
    *** ERROR: =back without previous =over  in file Koha/Plugins/Base.pm
    *** ERROR: empty =head2  in file Koha/Plugins/Base.pm
 * plugins/plugins-upload.pl                                               
FAIL
  critic                      FAIL
    Variable declared in conditional statement at line 62, column 5. Declare
variables outside of the condition.

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list