Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor $*KERNEL internal use of uname #2770

Merged
merged 1 commit into from Mar 17, 2019
Merged

Refactor $*KERNEL internal use of uname #2770

merged 1 commit into from Mar 17, 2019

Conversation

ugexe
Copy link
Member

@ugexe ugexe commented Mar 17, 2019

Uses nqp::uname() on MoarVM instead of shelling out, while shelling out on other backends.

Changes .release() and .version() to return more appropriate values -- uname version instead of uname release for .version(), and uname release instead of uname version for .release() 馃槙 By making these consistent with their uname counterpart people can better understand the values it may contain and by what name.

Uses `nqp::uname()` on MoarVM instead of shelling out, while shelling out on other backends.

Changes .release() and .version() to return more appropriate values -- uname version instead of uname release for .version(), and uname release instead of uname version for .release() 馃槙 By making these consistent with their uname counterpart people can better understand the values it may contain and by what name.
@ugexe
Copy link
Member Author

ugexe commented Mar 17, 2019

It would be nice if we could make .version() return a Str instead of a Version. If anything .release() should be a Version, but even then it doesn't entirely make sense to create the version object since the Version comparison operators on strings may not order themselves as Version expects.

@lizmat lizmat merged commit ffe8409 into master Mar 17, 2019
@lizmat
Copy link
Contributor

lizmat commented Mar 17, 2019

Thank you for your work and persistence :-)

@lizmat
Copy link
Contributor

lizmat commented Mar 17, 2019

Perhaps it makes sense to create a StrVersion allomorph?

@ugexe ugexe deleted the use-nqp-uname branch March 17, 2019 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants