search

fastmail / Moonpig

the moonpig billing system
69 stars 12 forks source link

Quickfixes for Moonpig::DateTIme #1

Closed mjdominus closed 10 years ago

mjdominus commented 11 years ago

Ported these bug fixes from DateTime::Moonpig.

I was not able to get the test t/datetime.t to run, so make sure the tests really pass.

mjdominus commented 10 years ago

The f8903e4 commit is to fix a large number of errors under 5.18.1 of this type:

Due to method name conflicts in roles 'Moonpig::Role::Consumer' and 'Moonpig::Role::Consumer::ByTime::FixedAmountCharge', the methods 'is_TimeInterval', 'to_Time', and 'to_TimeInterval' must be implemented or excluded by 'Moonpig::Class::Consumer::Consumer_ByTime_FixedAmountCharge'

I don't know what changed that causes these; it could be that namespace::autoclean used to pick up those functions and clean them, and doesn't any more, or it could be that something in the role composition code in Moose changed.

I don't like the fix, but I don't know what else to do.

mjdominus commented 10 years ago

I also have no idea why github thinks these two commits should be start of the same pull request. They weren't on the same branch. Does github assume that all pull requests from the same person should be joined into a single PR? If so, I think in the future I'll just mail a patch.

mjdominus commented 10 years ago

It's because I accidentally reused the branch name from 2 months ago, so GitHub decided it was related. Anyway, all the fixes in this PR were absorbed into DateTime::Moonpig, so this PR is now obsolete.