Add api_version argument and method for Net::Stripe object #130
Conversation
lib/Net/Stripe.pm
Outdated
| my $api_version = defined( $self->api_version ) ? $self->api_version : $self->_get_account_api_version(); | ||
|
|
||
| my @api_version = split( '-', $api_version ); | ||
| my $api_version_dt = eval { |
There was a problem hiding this comment.
I think this pattern is a bit risky in case the internals produce a list. I'd say it's safer to do:
my $api_version_dt;
eval {
$api_version_dt = DateTime->new(...);
};
There was a problem hiding this comment.
will adjust and revert.
lib/Net/Stripe.pm
Outdated
| die Net::Stripe::Error->new( | ||
| type => "API version validation error", | ||
| message => $message, | ||
| ) unless Scalar::Util::blessed( $api_version_dt ) && $api_version_dt->isa( 'DateTime' ); |
There was a problem hiding this comment.
DateTime is pretty strict and dies when the code at line 1736 doesn't produce a DateTime object, so I don't think you need this code. Unless there's a case you've found that it's necessary?
There was a problem hiding this comment.
i do not have an actual use case. i am a belt-and-suspenders kind of guy. :-)
lib/Net/Stripe/Constants.pm
Outdated
|
|
||
| use constant UNSUPPORTED_API_VERSION_PRE => '2012-09-24'; # valid API version, but less than min supported | ||
| use constant MIN_API_VERSION => '2012-10-26'; | ||
| use constant INVALID_API_VERSION => '2012-10-27'; # valid date within range, but invalid API version |
There was a problem hiding this comment.
INVALID_API_VERSION is only for testing, so I think it should be defined in the test script. Same for UNSUPPORTED_API_VERSION_PRE and UNSUPPORTED_API_VERSION_POST
There was a problem hiding this comment.
good catch. will fix and revert.
There was a problem hiding this comment.
any preference as to use constant vs my for these values within the test script?
also, a note as to the original reasoning for placement in Net::Stripe::Constants: even though not strictly necessary, the process of maintaining/updating these values, since they are all relative to one another, is eased by having them all in one place for easy visual comparison.
ie, as you update MIN_API_VERSION, you change UNSUPPORTED_API_VERSION_PRE to the previous value of MAX_API_VERSION, and you update INVALID_API_VERSION to the next calendar date after MIN_API_VERSION...
|
updates are ready for review |
sherrardb
force-pushed
the
master
branch
2 times, most recently
from
aec2a9f to
43da122
Compare
sherrardb
force-pushed
the
f-make-api-versionable
branch
from
6d0f238 to
bbe9fe0
Compare
* allow users to specify the API version when creating a new Net::Stripe object, per <https://stripe.com/docs/api/versioning#versioning> * add api_version argument and method for Net::Stripe object * add force_api_version argument and method for Net::Stripe object * update POD * closes <lukec#80> and <lukec#127>
sherrardb
force-pushed
the
f-make-api-versionable
branch
from
bbe9fe0 to
4fcbca2
Compare
allow users to specify the API version when creating a new Net::Stripe object, per https://stripe.com/docs/api#versioning