Skip to content

Conversation

@oWnOIzRi
Copy link
Contributor

enable warnings and use any instead of grep in boolean context

bin/asa Outdated

use strict;
use warnings;
use List::Util qw/ any /;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is if loading a module saves a significant amount of time over using a slightly inefficient grep. Can you benchmark both ways and see which one is faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading the module caused the 5.8 travis build to fail which I note is a requirement so I reverted back to the grep block form in the second commit. So ultimately the only thing that changed was I turned on warnings and changed the grep from expression to block form.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's odd because List::Util was core in 5.7.3.

For the warnings, I have the same comment as in #78. I'm not sure about shipping developer settings to end users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'any' is only listed as a suggested addition in 5.8 docs.

Let me know if you make a decision, I'm happy to help in any way I can.

@briandfoy
Copy link
Owner

So far we haven't added warnings to the scripts, so I'm deferring this.

@briandfoy briandfoy closed this Nov 5, 2019
@briandfoy briandfoy self-assigned this Jul 11, 2023
@briandfoy briandfoy added Status: released there is a new release with this fix Status: rejected this change is rejected Type: modernization updating programs to current practices Program: asa The asa program and removed Status: released there is a new release with this fix labels Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Program: asa The asa program Status: rejected this change is rejected Type: modernization updating programs to current practices

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants