Perl-Toolchain-Gang / CPAN-Meta-Requirements

a set of version requirements for a CPAN dist
5 stars 15 forks source link

Optimize merging Module => 0 into requirements #15

Closed Leont closed 9 years ago

Leont commented 9 years ago

This can be a rather significant bottleneck in merge heavy conditions such as an autoprereqs implementation.

dagolden commented 9 years ago

Most of the range code depends on versions being objects so that comparison functions do the right thing (and coerce accepts() operands).

I think that rather than ->with_minimum(0), it needs to be with_minimum(version->new(0), which should still be faster than the existing code.

Leont commented 9 years ago

Most of the range code depends on versions being objects so that comparison functions do the right thing (and coerce accepts() operands).

I think that rather than ->with_minimum(0), it needs to be with_minimum(version->new(0), which should still be faster than the existing code.

I didn't have that impression, and I didn't break any tests (in either CPAN::Meta::Requirements or CPAN::Meta). I guess more tests are needed.

dagolden commented 9 years ago

Agreed. Maybe accepts raw v strings. On Dec 17, 2014 7:32 AM, "Leon Timmermans" notifications@github.com wrote:

Most of the range code depends on versions being objects so that comparison functions do the right thing (and coerce accepts() operands).

I think that rather than ->with_minimum(0), it needs to be with_minimum(version->new(0), which should still be faster than the existing code.

I didn't have that impression, and I didn't break any tests (in either CPAN::Meta::Requirements or CPAN::Meta). I guess more tests are needed.

— Reply to this email directly or view it on GitHub https://github.com/dagolden/CPAN-Meta-Requirements/pull/15#issuecomment-67315825 .

dagolden commented 9 years ago

Fixed by #17