← Back to team overview

mvhub-dev team mailing list archive

Re: [Merge] lp:~leegoodrich/mvhub/create_config_in_mv_setup into lp:mvhub

 

Review: Resubmit
re subs using globals, I must have missed the parameters in merge diff. 

Closest I can see is:

    76  sub create_config_files_for{
    77          my $username = shift;
    78          my $CFG = shift;
   
...$CFG should be $cfg as it isn't really AN_ALL_CAPS_CONSTANT_DECLARED_AT_TOP_OF_FILE

#########
Another bit of slight ambiguity nit-picking really is:

  83          my $template_cfg = new Config::Simple($CFG->param('ABSOLUTE_PATH.template_conf_dir') . "/template.conf");  

This should probably be:

    $template_cfg_filename

to distinguish it from a template config object, which is how the variable is re-used elsewhere. 

Otherwise, fine & good, no need for more review after these fixes

-- 
https://code.launchpad.net/~leegoodrich/mvhub/create_config_in_mv_setup/+merge/22003
Your team mvhub-dev is subscribed to branch lp:mvhub.



References