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

Matt S Trout mst at shadowcat.co.uk
Tue Jan 24 22:47:07 GMT 2017


On Tue, Jan 17, 2017 at 10:20:13PM -0600, Mike South wrote:
> 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".

Ah, yes, one of those silly mistakes that's easy to make when you're mid
development.

Definitely worth unifying one way or the other; I'm untroubled as to which.

Should also probably be a way to trigger the version check automatically.

My suggestion would be for the plugin to wrap connection() to take a
version_check (or check_version ;) attribute in the final hashref, plus
a class accessor that provides a default which the attribute overrides if
present.

> 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.

How about on_database_too_new and is_database_too_new as a naming pattern?
 
> 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.

If you want to test an API for intuitiveness, a relative newcomer is the
best sort of test case, tbh. It might be important that the implementation
makes sense to me, but where the API and the docs are concerned whether they
make sense to you is almost certainly a better measure of "is this sane?" :)

-- 
Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit and a clue

http://shadowcat.co.uk/blog/matt-s-trout/   http://twitter.com/shadowcat_mst/

Email me now on mst (at) shadowcat.co.uk and let's chat about how our CPAN
commercial support, training and consultancy packages could help your team.



More information about the DBIx-Class mailing list