kentnl / App-DH

Deploy your DBIx::Class Schema to DDL/Database via DBIx::Class::DeploymentHandler
Other
0 stars 1 forks source link

can specify target version for installs/upgrades #1

Closed yanick closed 9 years ago

kentfredric commented 9 years ago
# Failed test 'Test::Perl::Critic for "blib/lib/App/DH.pm"'
# at /home/travis/perl5/perlbrew/perls/5.21/lib/site_perl/5.21.0/Test/Perl/Critic.pm line 104.
#
# [CodeLayout::ProhibitTrailingWhitespace] Found "\N{SPACE}" at the end of the line at line 261, near 'sub cmd_upgrade { '. (Severity: 1)
# [CodeLayout::ProhibitTrailingWhitespace] Found "\N{SPACE}" at the end of the line at line 264, near '$self->_dh->upgrade({ to_version => $self->target }); '. (Severity: 1)
# [CodeLayout::ProhibitTrailingWhitespace] Found "\N{SPACE}" at the end of the line at line 267, near '$self->_dh->upgrade; '. (Severity: 1)
[16:20:30] ./xt/author/critic.t ............ Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests
[16:20:33] ./xt/release/kwalitee.t ......... These tests should not be running unless AUTHOR_TESTING=1 and/or RELEASE_TESTING=1!
[16:20:33] ./xt/release/kwalitee.t ......... ok 847 ms
[16:20:34] ./xt/release/pod-coverage.t ..... ok 1649 ms
[16:20:35] ./xt/release/distmeta.t ......... ok 373 ms
[16:20:36] ./xt/release/pod-syntax.t ....... ok 409 ms
[16:20:36] ./xt/author/eol.t ............... 1/?
# Failed test 'No incorrect line endings in 'lib/App/DH.pm' on line 261: [\s] '
# at ./xt/author/eol.t line 16.
# Problem Lines:
# line 261: [\s] : sub[\s]cmd_upgrade[\s]{[\s]
# Looks like you failed 1 test of 4.
kentfredric commented 9 years ago

Another stylistic nitpick:

if ( $self->has_target ) {
 $self->_dh->install({ version => $self->target });
}
else {
 $self->_dh->install;
}
return;

I'd probably prefer that structure represented as:

if ( $self->has_target ) {
 $self->_dh->install({ version => $self->target });
 return;
}
$self->_dh->install;
return;

Or perhaps:

my $opts = {};
$opts->{version} = $self->target if $self->has_target;
$self->_dh->install($opts);
return;

Assuming of course that the empty hash has the same effect as no arguments, which seems to be true for ->install(): https://metacpan.org/source/FREW/DBIx-Class-DeploymentHandler-0.002215/lib/DBIx/Class/DeploymentHandler/Dad.pm#L38

However, seems ->upgrade() doesn't take any arguments at present: https://metacpan.org/source/FREW/DBIx-Class-DeploymentHandler-0.002215/lib/DBIx/Class/DeploymentHandler/Dad.pm#L56

yanick commented 9 years ago

I've simplified the --target stuff, and added tests!