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

deps: accept formula options. #5648

Merged
merged 1 commit into from
Jan 30, 2019
Merged

deps: accept formula options. #5648

merged 1 commit into from
Jan 30, 2019

Conversation

MikeMcQuaid
Copy link
Member

Also, cleanup usage of <formula>.

As requested in comments of #5587.

CC @RandomDSdevel @blogabe

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Also, cleanup usage of `<formula>`.

As requested in comments of #5587.
@blogabe
Copy link

blogabe commented Jan 30, 2019

cheers @MikeMcQuaid

@RandomDSdevel
Copy link
Contributor

RandomDSdevel commented Jan 30, 2019

     Sorry for not acting on this, @MikeMcQuaid; I was waiting to see whether @blogabe was planning to tackle this or not.

@MikeMcQuaid
Copy link
Member Author

No worries @RandomDSdevel!

@MikeMcQuaid MikeMcQuaid merged commit 2062c51 into Homebrew:master Jan 30, 2019
@MikeMcQuaid MikeMcQuaid deleted the deps-formula-options branch January 30, 2019 16:32
@blogabe
Copy link

blogabe commented Feb 22, 2019

Hi guys!

I'm still confused about what this change was supposed to fix. Hoping you can help me understand. Prior to #5587, I was able to execute this command ( brew deps -n --include-build --include-requirements --include-optional <formula> <option1> <option 2> ) to get a list of the dependencies the as required by the formula and the options entered on the command line. Obviously this now only pertains to formulae on personal taps.

Since the above change and even after this change, the above command will print ALL optional dependencies effectively ignoring the options passed on the command line.

So if the formula by itself has no dependencies, option 1 depends on dep1, option 2 on dep2, and option 3 on dep3, I would expect the command to output dep1 and dep2 per the historical usage. but now it outputs dep1, 2, and 3 even though option 3 was never passed.

User error on my part? Am I misunderstanding what this change was supposed to do?

Mike, what happened to the other picture with your dog???

Thanks for your help guys. Appreciate it.

@blogabe
Copy link

blogabe commented Feb 22, 2019

Geez... I'm an idiot. Started messing around and realized the command does what I want if I simply remove the --include-optional parameter. Feel free to disregard my prior message.

@lock lock bot added the outdated PR was locked due to age label Mar 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants