Skip to content

Make HOMEBREW_PREFIX/Caskroom the default.#1025

Merged
reitermarkus merged 2 commits intoHomebrew:masterfrom
reitermarkus:migrate-caskroom
Sep 19, 2016
Merged

Make HOMEBREW_PREFIX/Caskroom the default.#1025
reitermarkus merged 2 commits intoHomebrew:masterfrom
reitermarkus:migrate-caskroom

Conversation

@reitermarkus
Copy link
Copy Markdown
Member

@reitermarkus reitermarkus commented Sep 19, 2016

  • 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 tests with your changes locally?

Fixes Homebrew/homebrew-cask#24610.

@reitermarkus reitermarkus added the in progress Maintainers are working on this label Sep 19, 2016
@MikeMcQuaid
Copy link
Copy Markdown
Member

@reitermarkus May also be worth tweaking https://github.com/Homebrew/brew/blob/master/Library/Homebrew/cmd/update-report.rb#L234-L239. Happy to merge this once we have one job with 👍 to avoid waiting for coverage completion.

@MikeMcQuaid
Copy link
Copy Markdown
Member

Also: will this break any symlinks?

@reitermarkus
Copy link
Copy Markdown
Member Author

reitermarkus commented Sep 19, 2016

I'm not sure about tweaking update-report.rb, because if the Caskroom isn't moved to the new repository first, there won't be a reference to it.

@MikeMcQuaid
Copy link
Copy Markdown
Member

It feels pretty odd to move it and then move it back to the same location?

Comment thread Library/Homebrew/cmd/update-report.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is only run if HOMEBREW_REPOSITORY and HOMEBREW_PREFIX are both /usr/local so it's a no-op and can be removed.

Comment thread Library/Homebrew/cmd/update-report.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These will always be the same path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, ok, will remove this whole section then.

@reitermarkus
Copy link
Copy Markdown
Member Author

Also: will this break any symlinks?

It might. But it will probably fix more symlinks than it breaks, since the move to /usr/local/Homebrew/Caskroom broke all links pointing to /usr/local/Caskroom.

@jawshooah
Copy link
Copy Markdown
Contributor

@reitermarkus commented on Sep 19, 2016, 10:16 AM EDT:

Also: will this break any symlinks?

It might. But it will probably fix more symlinks than it breaks, since the move to /usr/local/Homebrew/Caskroom broke all links pointing to /usr/local/Caskroom.

It will certainly break symlinks, but I'm not sure how it would fix any.

@reitermarkus
Copy link
Copy Markdown
Member Author

reitermarkus commented Sep 19, 2016

/usr/local/Caskroom was migrated to /usr/local/Homebrew/Caskroom with the migration of the Homebrew repo to /usr/local/Homebrew, so all symlinks would be broken if we left the Caskroom at /usr/local/Homebrew/Caskroom.

@jawshooah
Copy link
Copy Markdown
Contributor

Ah, sorry, read your comment too fast and thought you were talking about the earlier migration from /opt/homebrew-cask/Caskroom to /usr/local/Caskroom.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 19, 2016

Current coverage is 81.76% (diff: 100%)

Merging #1025 into master will decrease coverage by 1.52%

@@             master      #1025   diff @@
==========================================
  Files           340        340          
  Lines         14109      14390   +281   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          11751      11766    +15   
- Misses         2358       2624   +266   
  Partials          0          0          

Powered by Codecov. Last update 3ffb9a2...c0cc703

@reitermarkus reitermarkus merged commit e22610a into Homebrew:master Sep 19, 2016
@reitermarkus reitermarkus deleted the migrate-caskroom branch September 19, 2016 17:08
@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

in progress Maintainers are working on this

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants