Skip to content

brew: remove sudo call#5917

Merged
iMichka merged 1 commit intoHomebrew:masterfrom
iMichka:sudo
Mar 24, 2019
Merged

brew: remove sudo call#5917
iMichka merged 1 commit intoHomebrew:masterfrom
iMichka:sudo

Conversation

@iMichka
Copy link
Copy Markdown
Member

@iMichka iMichka commented Mar 23, 2019

Closes #5865

  • 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?

Comment thread Library/Homebrew/brew.rb Outdated
Comment thread Library/Homebrew/brew.rb Outdated
@iMichka iMichka requested a review from vitorgalvao March 24, 2019 14:50
@iMichka iMichka self-assigned this Mar 24, 2019
@iMichka iMichka force-pushed the sudo branch 2 times, most recently from 476a468 to f31bfed Compare March 24, 2019 15:54
@sjackman
Copy link
Copy Markdown
Contributor

+ sudo -E -u linuxbrew /home/linuxbrew/.linuxbrew/bin/brew test-bot
Error: Invalid tap name '{"name"=>"homebrew/test-bot", "user"=>"Homebrew", "repo"=>"test-bot", "path"=>"/home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-test-bot", "installed"=>false, "official"=>true, "formula_names"=>[], "formula_files"=>[], "command_files"=>[], "pinned"=>false}'
Error: Failure while executing; `/home/linuxbrew/.linuxbrew/bin/brew tap \{\"name\"=\>\"homebrew/test-bot\",\ \"user\"=\>\"Homebrew\",\ \"repo\"=\>\"test-bot\",\ \"path\"=\>\"/home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-test-bot\",\ \"installed\"=\>false,\ \"official\"=\>true,\ \"formula_names\"=\>\[\],\ \"formula_files\"=\>\[\],\ \"command_files\"=\>\[\],\ \"pinned\"=\>false\}` exited with 1.

Odd. I guess it needs possible_tap.name. Odd that it worked before. #{possible_tap} may be equivalent to possible_tap.to_s. It looks like we're getting possible_tap.inspect for some reason now.

Comment thread Library/Homebrew/brew.rb Outdated
@iMichka
Copy link
Copy Markdown
Member Author

iMichka commented Mar 24, 2019

Odd. I guess it needs possible_tap.name. Odd that it worked before. #{possible_tap} may be equivalent to possible_tap.to_s. It looks like we're getting possible_tap.inspect for some reason now.

That's because each time I try to change something in brew, I stumble over weird things :)

Anyway, it is better to call .name, instead of relying on the .to_s implementation which may change.

Copy link
Copy Markdown
Contributor

@sjackman sjackman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Michka!

@iMichka iMichka merged commit eece1f0 into Homebrew:master Mar 24, 2019
@iMichka iMichka deleted the sudo branch March 24, 2019 17:15
@MikeMcQuaid
Copy link
Copy Markdown
Member

Good call, thanks @iMichka!

@sjackman
Copy link
Copy Markdown
Contributor

This PR was reverted in PR #6010.

@iMichka
Copy link
Copy Markdown
Member Author

iMichka commented Apr 16, 2019

cc @jonchang

@lock lock bot added the outdated PR was locked due to age label May 16, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 16, 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.

3 participants