[Dbix-class] Please review my proposed CPAN Module: DBIx-Class-Schema-VersionCheck

[Mike South]

Hi Daniel,

One comment I would make is that I think you should pick one word order and
stick with it for everything.  Your module on github is named CheckVersion;
the perl code inside of it is named VersionCheck.pm; the subroutine is
named "check_version"; the environment variable is named
"DBIC_SKIP_VERSION_CHECK".

It's weird to me to have a subroutine named with a leading underscore that
is designed to be overridden (I'm new to the guts of DBIC components, so if
that's normal, ignore my ignorant input).

The naming of those subroutines "database_too_new" and "database_too_old"
make them sound like booleans--as if you would be able to do this:

if ($schema->database_too_new) {
# deal with that
}
elsif ( $schema->database_too_old ) {
# deal with that
}

Maybe they should be named "handle_database_too_old" and
"handle_database_too_new".  I would also suggest that you actually
implement database_too_new and database_too_old and make those available to
the user as well.

For example, a user may want to check if the database is too old, and, if
so, attempt to apply an update script, and then check again, or something
like that, rather than only having the ability to specify a particular
subroutine to run in the database-too-old situation.

I very much like your idea of having (what appears to me to be) a clean
wrapper that can be called by the user in one function.  Having said that,
I'm a relative newcomer to DBIC and I've never used DeploymentHandler, so
my input is certainly less informed than it could be.

mike


On Fri, Jan 6, 2017 at 6:58 AM, Daniel Böhmer <post at daniel-boehmer.de>
wrote:

> Hello DBIC devs and users, escpecially fREW,
>
> I poured some work of mine into a dedicated Perl distribution and think
> about
> uploading it to CPAN. My hope is to get some feedback from you.
>
> Currently you can find the code only on Github:
> https://github.com/dboehmer/DBIx-Class-Schema-CheckVersion
>
> Before uploading it to CPAN in the first place I'd like to know:
>
> - Are there any existing modules doing the same job?
>
> - Before occupying the namespace: Do have any thoughts on the name?
>
> Even after uploading I am interested in:
>
> - bug reports
>
> - feature requests
>
> - patches in form of Github Pull Requests
>
> - ideas how to enable automatic calling of check_version() during connect()
> but still allowing deploying and therefor connecting to any empty DB w/o
> check
>
> Here's the story of what the module is about:
>
> I am adopting DBIC::DeploymentHandler in a project I'm currently working
> for.
> We agreed on having having a check on server startup to see if the database
> has the same version as our schema. It wasn't trivial for me to find out
> how
> to do that but the result are a few pretty generic lines. You can inherit
> the module from your schema class and call check_version()
>
> I am looking forward to your feedback and releasing the module.
>
> Kind regards
> Daniel Böhmer aka DBOEHMER
>
> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive: http://www.grokbase.com/group/
> dbix-class at lists.scsys.co.uk
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20170117/54eee772/attachment-0001.htm>