Skip to content

Extract runtime detection constants into separated module #138

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Extract runtime detection constants into separated module #138

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3370594
Move runtime detections into dedicated library
happy-barney Mar 18, 2025
c549ab7
Use runtime constants in tests as well
happy-barney Mar 18, 2025
8a44bd2
fixup! Move runtime detections into dedicated library
happy-barney Mar 19, 2025
033f677
fixup! Move runtime detections into dedicated library
happy-barney Mar 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/TAP/Harness/Runtime.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use warnings;

use base q (Exporter);

use constant HAS_TIME_HIRES => !! eval { use Time::HiRes qw (time); 1 };
use constant HAS_TIME_HIRES => !! eval q { use Time::HiRes qw (time); 1 };
Copy link
Member

@karenetheridge karenetheridge Mar 22, 2025

Choose a reason for hiding this comment

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

This changes the eval from a block style evaluation to a string, which is not a good change.

Copy link
Contributor

@Grinnz Grinnz Mar 22, 2025

Choose a reason for hiding this comment

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

Block eval would not be able to test a use statement, but this can easily be done with a block eval: eval { require Time::HiRes; Time::HiRes->import('time'); 1 }

Copy link
Author

@happy-barney happy-barney Mar 22, 2025

Choose a reason for hiding this comment

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

@karenetheridge original code had string evaluation.

I'm using form q { } for string containing code (hence { as paired delimiter).

commit introducing this change is fixup! of original commit. I'm using such approach to show reaction to code review comment, referring fixup commit(s) dealing with given issue raised in given comment.
(when I was taught this approach I hesitated but at the end, in many PR-review cycles, I found out that review by commit and reacting with fixups saved lot of time especially on reviewer's side)

@Grinnz my aim was only to extract existing runtime detection into dedicated library, but your suggestion is too good to be omitted ... 033f677

use constant IS_VMS => $^O eq q (VMS);
use constant IS_WIN32 => ($^O =~ qr (^ (MS) Win32 $)x);
use constant IS_UNIXY => ! (IS_VMS || IS_WIN32);
Expand Down