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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Update 'windows_os()' helper to use PHP_OS_FAMILY #30660

Merged
merged 1 commit into from
Nov 22, 2019

Conversation

owenvoke
Copy link
Contributor

Originally opened against 7.x, see #30658.

Since Laravel 6+ depends on PHP 7.2 or later, the PHP_OS_FAMILY constant is available.

Whereas the PHP_OS constant can be WINNT, WIN32, Windows, etc., the PHP_OS_FAMILY will always be Windows if PHP is built for Windows (and has a predefined list of available values).

The performance of PHP_OS_FAMILY is a micro-optimisation, compared to the existing PHP_OS usage (except for on PHP 7.1, which isn't supported by Laravel 6), but it seems to be handled better when used in iterations. However, the existing method works fine, so I understand this probably won't be merged. I'd be interested to hear peoples opinions on using this over the older method. 👍

I've tested this in Windows, Windows Subsystem for Linux (WSL), MinGW, MacOS, and Ubuntu.

@owenvoke owenvoke changed the title Update 'windows_os()' helper to use PHP_OS_FAMILY [6.x] Update 'windows_os()' helper to use PHP_OS_FAMILY Nov 22, 2019
@taylorotwell taylorotwell merged commit 3e4c2c6 into laravel:6.x Nov 22, 2019
@owenvoke owenvoke deleted the feature/windows-os-helper branch November 22, 2019 20:20
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.

2 participants