-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Expose uses_from_macos
list in formula API
#6467
Expose uses_from_macos
list in formula API
#6467
Conversation
ac427d6
to
7419bb4
Compare
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.
Implementation looks good but just have some thoughts on naming.
c1fbdd8
to
13255ec
Compare
uses_from_macos
in formula API as "macos_dependencies"uses_from_macos
list in formula API
13255ec
to
b6efd65
Compare
@@ -4,22 +4,28 @@ class SoftwareSpec | |||
undef uses_from_macos | |||
|
|||
def uses_from_macos(deps, **args) | |||
@macos_deps ||= [] |
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.
Thoughts on just uses_from_macos
? I think the use of deps
here is a bit misleading because at this stage this aren't actually dependencies.
b6efd65
to
975c4e4
Compare
975c4e4
to
f6ef26a
Compare
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.
Sorry, missed the updates here! Give me a shout if you force-push and I don't acknowledge it. Thanks again @EricFromCanada!
brew style
with your changes locally?brew tests
with your changes locally?As suggested in #6162.
This also fixes some inverted logic in how
before:
andafter:
are handled, as mentioned earlier, i.e.add_mac_dependency?
needs to return false (causing the dependency version shipped with macOS to be used) if the current OS version is greater than or equal to theafter:
value, or less than thebefore:
value.