-
Notifications
You must be signed in to change notification settings - Fork 76
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
Correct test description. #294
Conversation
I believe the correct thing to do is to reverse the read only and write only checks, so that the rm_rf test description is correct. That's what this was originally intended to check (fa708b9), but it somehow got lost in the shuffle between EU::Command, EUMM, and blead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per previous comment.
Per code review by haarg, the order of tests was wrong in the first place. Hence, correctly re-ordering them is a better repair. Perl-Toolchain-Gang#294 (review)
On 05/09/2017 03:35 AM, Graham Knop wrote:
I believe the correct thing to do is to reverse the read only and write
only checks, so that the rm_rf is test description is correct. That's
what this was originally intended to check (fa708b9
<fa708b9>),
but it somehow got lost in the shuffle between EU::Command, EUMM, and blead.
Done. Please re-review.
Thank you very much.
Jim Keenan
|
Could you squash these into a single commit? Other than that, looks good. |
Per code review by haarg, the order of tests was wrong in the first place. Hence, correctly re-ordering them is a better repair than changing one test's description. For: Perl-Toolchain-Gang#294
On 05/09/2017 10:18 AM, Graham Knop wrote:
Could you squash these into a single commit? Other than that, looks good.
Pushed.
|
If this pull request is satisfactory, I would request a merge into master and timely release of a new version of ExtUtils-MakeMaker to CPAN. I have to soon release a new version of File-Path. It's doing fine on CPANtesters, but there would be one failure in ExtUtils-MakeMaker's t/eu_command.t in the absence of a new release of the latter. Thank you very much. Jim Keenan |
Per code review by haarg, the order of tests was wrong in the first place. Hence, correctly re-ordering them is a better repair than changing one test's description. For: Perl-Toolchain-Gang/ExtUtils-MakeMaker#294 [Debian note: this is a prerequisite for the CVE-2017-6512 fix in File-Path] Bug: Perl-Toolchain-Gang/ExtUtils-MakeMaker#294 Patch-Name: fixes/extutils_file_path_compat.diff
Per code review by haarg, the order of tests was wrong in the first place. Hence, correctly re-ordering them is a better repair than changing one test's description. For: Perl-Toolchain-Gang/ExtUtils-MakeMaker#294 [Debian note: this is a prerequisite for the CVE-2017-6512 fix in File-Path] Bug: Perl-Toolchain-Gang/ExtUtils-MakeMaker#294 Patch-Name: fixes/extutils_file_path_compat.diff
Per code review by haarg, the order of tests was wrong in the first place. Hence, correctly re-ordering them is a better repair than changing one test's description. For: Perl-Toolchain-Gang/ExtUtils-MakeMaker#294 [Debian note: this is a prerequisite for the CVE-2017-6512 fix in File-Path, and was backported by Dominic Hargreaves] Bug: Perl-Toolchain-Gang/ExtUtils-MakeMaker#294 Patch-Name: fixes/extutils_file_path_compat.diff
At this point, 'testdir' is write-only (0200), not read-only.