-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
refactor: drop need for an obscure (and legacy) OC_Util method... runningOnMac() ;-)
#56816
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
Conversation
- Easy enough query directly these days; Only used in one spot anyhow - Updated the runningOnMac method to use PHP_OS_FAMILY just for clarity until we can remove the function entirely (same result; supported since PHP ~7.4). Signed-off-by: Josh <[email protected]>
Use PHP's newer PHP_OS_FAMILY constant. - Allows us to drop a legacy OC_Util method. - This test isn't even enabled at the moment. Signed-off-by: Josh <[email protected]>
PHP_OS_FAMILY - Facilitates elimination of a legacy OC_Util method - `runningOnMac()` -- yes, really! ;-) - Supported since PHP late 7.x-era Signed-off-by: Josh <[email protected]>
CarlSchwan
left a comment
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.
One less :)
Signed-off-by: Carl Schwan <[email protected]>
|
Doc part nextcloud/documentation#13907 |
szaimen
left a comment
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.
LGTM but did not test
Removed deprecated method 'runningOnMac' that checks if PHP is running on macOS. Signed-off-by: Josh <[email protected]>
Signed-off-by: Josh <[email protected]>
|
This breaks MacOS CI as the error is now not ignored anymore on install: https://github.com/nextcloud/talk-ios/actions/runs/20001229346/job/57357971111?pr=2306 Needs adjustment at:
(Leaving out the question if there’s a better way to do this ..) |
|
Since we don't support macOS, you should also not use it in your CI this way 🙈 The right way is probably using a docker container for this in CI |
|
Since macOS runner don’t support docker or nested virtualisation, this is the only way right now. |
Summary
TODO
Checklist
3. to review, feature component)stable32)Footnotes
Once we feel comfortable removing it. Not really sure if it's considered a public API or not. I guess kinda sorta. So
@deprecatedit (and changed its implementation to use the constant meanwhile too). ↩