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

Gate MacOS calls behind OS.mac? conditionals #984

Merged
merged 1 commit into from
Nov 21, 2023
Merged

Gate MacOS calls behind OS.mac? conditionals #984

merged 1 commit into from
Nov 21, 2023

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Nov 21, 2023

See Homebrew/brew#16224

This PR makes it so that all references to the MacOS module are gates behind OS.mac? calls to allow the MacOS module to be deprecated on Linux.

My only outstanding question is whether this reference needs to be changed:

module MacOS
module_function
def version
:catalina
end
end

This is in the test directory so it shouldn't affect functionality, only CI in this repo.

@Bo98
Copy link
Member

Bo98 commented Nov 21, 2023

My only outstanding question is whether this reference needs to be changed:

It doesn't need to be changed, no. You could make it conditional to Darwin to force it to error on Linux if encountered, but that doesn't matter much.

The design of the rspecs were aimed to test test-bot without a brew install, so we have stubbed versions of os.rb.

The rspecs in this repo is very poor anyway atm and hardly do anything. It certainly won't be hitting this sudo purge env line yet.

@Bo98 Bo98 merged commit eca29a3 into Homebrew:master Nov 21, 2023
4 checks passed
@Rylan12 Rylan12 deleted the remove-macos-on-linux branch November 21, 2023 01:19
@MikeMcQuaid
Copy link
Member

Thanks @Rylan12 for the PR and @Bo98 for the review and merge!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants