-
-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Install size #19254
base: master
Are you sure you want to change the base?
Install size #19254
Conversation
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.
Thanks @tyuwags, great work so far!
… and adding parameter
Adding the |
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.
Looking good so far, thanks @tyuwags! Remaining bits would be:
- extracting into shared methods
- adding a unit test for the extracted method to get some code coverage for this
Shout if you need help with any of that!
@@ -302,6 +307,98 @@ def run | |||
Install.perform_preinstall_checks_once | |||
Install.check_cc_argv(args.cc) | |||
|
|||
ask_input = lambda { |
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.
Can these be dedicated methods rather than a lambda? The method this is part of is getting very long and we don't typically use lambda
like this just for variable scoping.
@@ -126,6 +131,98 @@ def run | |||
unless formulae.empty? | |||
Install.perform_preinstall_checks_once | |||
|
|||
ask_input = lambda { | |||
ohai "Do you want to proceed with the installation? [Y/y/yes/N/n]" |
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.
Seems like a lot of duplicated code here, would be good to extract into shared method(s) in Library/Homebrew/install.rb
For the shared library: |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?resolve #18373
As discussed in the issue, these changes add the
--ask
option and theHOMEBREW_ASK
environment variable to display the packages and dependencies that will be installed, along with their sizes, and prompt the user for input.