New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace string eval with block eval #9

Merged
merged 1 commit into from Feb 7, 2015

Conversation

Projects
None yet
2 participants
@paultcochrane
Contributor

paultcochrane commented Feb 6, 2015

According to Perl Best Practices, string eval should be avoided, hence
this change.

Paul Cochrane
Replace string eval with block eval
According to Perl Best Practices, string `eval` should be avoided, hence
this change.
@hoehrmann

This comment has been minimized.

Owner

hoehrmann commented Feb 7, 2015

The use of string eval for this purpose comes straight from the documentation of the two testing modules used, and I am not sure it would be correct to replace it with a block eval (note that if the used module is not available, the tests should not fail and not generate warnings). So I think I'll keep it that way. If there is no problem using block eval, it would be nice if the documentation for the two testing modules got updated.

@paultcochrane

This comment has been minimized.

Contributor

paultcochrane commented Feb 7, 2015

I discussed this issue with the the current maintainer of Test::Pod::Coverage on IRC and he recommended using the block form of eval. I'm about to patch the Test::Pod::Coverage docs to reflect this, so a future release of T::P::C should have the up to date documentation.

hoehrmann added a commit that referenced this pull request Feb 7, 2015

@hoehrmann hoehrmann merged commit f753afc into hoehrmann:master Feb 7, 2015

@hoehrmann

This comment has been minimized.

Owner

hoehrmann commented Feb 7, 2015

Very well then. The other module with the same documentation flaw was Test::Pod I think. Thanks!

@paultcochrane paultcochrane deleted the paultcochrane:pr/avoid_string_eval branch Feb 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment