search

Dual-Life / Time-Piece

Object Oriented time objects
Other
15 stars 33 forks source link

Add new method "truncate" for RT 62189 #25

Closed openstrike closed 7 years ago

openstrike commented 7 years ago

Here is an implementation of the proposed "truncate" method discussed in RT 62189. This is not a full replacement for the truncate method of DateTime because it does not handle either of the "week" or "local_week" options. It does cover all the other possible values of the "to" parameter, however.

There's a test script and a documentation update included.

Time::Piece is my module for this month's CPAN PR Challenge - thanks for participating!

smith153 commented 7 years ago

Looks like a cool addition! My only concern is the use of Test::Trap. I don't think Test::Trap is included in core? And as such, someone building from source wouldn't be able to run that test (as Test::Trap wouldn't be available). Perhaps one could rewrite it by catching it with eval instead?

openstrike commented 7 years ago

No, Test::Trap isn't core. I've used it quite a bit previously and found it to be a simple way to test for exceptions. By couching it in the SKIP block it means that the tests shouldn't fail because of the absence of the module but as you say neither would they be run. It is very easy to replace with plain evals which has been done in the commit b6f2a73 just pushed.

Thanks for reviewing.

openstrike commented 7 years ago

Are you happy with the amended PR which removes Test::Trap, @smith153? Are there any further modifications which you would like before merging?

smith153 commented 7 years ago

Everything looks good to me. I tested it out a month ago or so and didn't see any issues. I should be merging this in next week and pushing out a new release :)

smith153 commented 7 years ago

Merged :)