search

Perl-Critic / PPI

54 stars 44 forks source link

remove remaining usage of List::MoreUtils #205

Closed Grinnz closed 7 years ago

Grinnz commented 7 years ago

The remaining usage of List::MoreUtils can be easily replaced with any and first from List::Util. The main difference between first and firstidx is that firstidx returns -1 if the element is not found, and first returns undef, but for the majority of these instances, this case is not currently checked for anyway -- it is assumed the document is consistent with itself so the node will be found.

Grinnz commented 7 years ago

Rebased

moregan commented 7 years ago

This change specifies the relatively-recent List::Util 1.33, which might not be compatible with running under Perl 5.6.x: http://matrix.cpantesters.org/?dist=Scalar-List-Utils%201.33 (I'm not at all against dropping 5.6 support, but I haven't noticed that that has happened.)

adamkennedy commented 7 years ago

I doubt there's any good reason to retain 5.6 support any more.

Adam

On Fri, 30 Jun 2017 at 7:45 am, moregan notifications@github.com wrote:

This change specifies the relatively-recent List::Util 1.33, which might not be compatible with running under Perl 5.6.x: http://matrix.cpantesters.org/?dist=Scalar-List-Utils%201.33 (I'm not at all against dropping 5.6 support, but I haven't noticed that that has happened.)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/adamkennedy/PPI/pull/205#issuecomment-312286559, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHA_h4xQM8bU9vYOiqK21AiAKOIqx-Iks5sJQoAgaJpZM4NvbZM .

Grinnz commented 7 years ago

List::Util 1.33 is already specified as a prerequisite of PPI, there's no change there. For what it's worth, List::Util does mostly work on 5.6, just some tests fail so it has to be force installed. https://github.com/Dual-Life/Scalar-List-Utils/pull/29

Grinnz commented 7 years ago

Actually List::MoreUtils requires 5.8 so this change is required for 5.6 support ;)

wchristian commented 7 years ago

@Grinnz Thanks a bunch, cherry-picked on top of master. :)

adamkennedy commented 7 years ago

Hah, sorry :)

Who's asking for 5.6 support? Old HPUX servers?

Adam

On Fri, 30 Jun 2017 at 9:58 am, Dan Book notifications@github.com wrote:

Actually List::MoreUtils requires 5.8 so this change is required for 5.6 support ;)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/adamkennedy/PPI/pull/205#issuecomment-312319965, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHA_j90IcTVumJZAcsANG3-qEyaJGMSks5sJSkcgaJpZM4NvbZM .

wchristian commented 7 years ago

@adamkennedy I would definitely like to get it to that place if we can make it without sacrifice. :)