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.

Sign up for GitHub

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

Jump to bottom

Use ~/Library/Caches as default cache location on OSX #1638

Merged
merged 1 commit into from
Nov 3, 2016
Merged

Use ~/Library/Caches as default cache location on OSX #1638

merged 1 commit into from
Nov 3, 2016

Conversation

sebmck
Copy link
Contributor

@sebmck sebmck commented Nov 3, 2016

No description provided.

@sebmck sebmck mentioned this pull request Nov 3, 2016
Closed
@bestander
Copy link
Member

So does npm.

bestander
bestander approved these changes Nov 3, 2016
View reviewed changes
Copy link
Member

@bestander bestander left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks reasonable to be in line with OS specifics.
Just a few concerns for the future:

  • npm and many other apps use home folder anyway
  • next, Windows and Linux users will ask for their cache folder locations to be satisfied
  • we need to let users know in change details, people may depend on ~/.yarn-cache in their routines and integrations

@sebmck
Copy link
Contributor Author

sebmck commented Nov 3, 2016

@bestander

next, Windows and Linux users will ask for their cache folder locations to be satisfied

This seems fine. We already use %APPDATA% on Windows (as does npm). It makes sense to be a good citizen on the platforms we support and put our files in the correct places so tooling works correctly.

@sebmck sebmck merged commit eafc401 into master Nov 3, 2016
@sebmck sebmck deleted the darwin-cache branch November 3, 2016 10:48
Jessidhia pushed a commit to Jessidhia/yarn that referenced this pull request Nov 4, 2016
Merge branch 'master' into npm4-prepublish
fda0ecf 
* master: (66 commits)
  Add --no-bin-links flag - fixes yarnpkg#929 (yarnpkg#1651)
  Add option to change the prefix of the global bin folder - fixes yarnpkg#630 (yarnpkg#1654)
  patterns -> filteredPatterns
  Add helpful nudge to yarnpkg/rfcs on issue template (yarnpkg#1650)
  Change reporter.log to console.log in generate-lock-entry command - fixes yarnpkg#644 (yarnpkg#1652)
  Fixed add command flag (yarnpkg#1653)
  Nested executables fix (yarnpkg#1210)
  Added short-flags for yarn add (yarnpkg#1618)
  Add name lookups to ls command - fixes yarnpkg#1599 (yarnpkg#1643)
  Disable flaky secureUrl test (yarnpkg#1644)
  Add unit tests for `yarn why`. (yarnpkg#1544)
  Refine flow type for config.generateHardModulePath (yarnpkg#1642)
  Use ~/Library/Caches as default cache location on OSX - fixes yarnpkg#1637 (yarnpkg#1638)
  Update aliases.js (yarnpkg#1635)
  Update aliases.js (yarnpkg#1634)
  Add webhook to archive AppVeyor build artifacts (yarnpkg#1631)
  Attempt to fix failing Circle CI builds (yarnpkg#1629)
  Adding 'yarn global upgrade'(Issue yarnpkg#776) (yarnpkg#1616)
  Show error message in stdout. (yarnpkg#1502)
  Nicer permission errors when trying to write global binaries - fixes yarnpkg#1578 (yarnpkg#1592)
  ...
@Daniel15 Daniel15 mentioned this pull request Nov 7, 2016
Closed
@Daniel15
Copy link
Member

Daniel15 commented Nov 7, 2016

people may depend on ~/.yarn-cache in their routines and integrations

Cache directories are an internal implementation detail, I don't think anyone should rely on them (or they should use a Yarn API to get the path)

I agree that we should use the right path on Linux though - /var/cache/ for root, ~/.cache/ for regular users.

@hpurmann
Copy link
Contributor

@Daniel15 It's pretty helpful to know where the cache lives in case you want to mount a Docker volume containing it.

VOLUME /root/.yarn-cache

IMHO this change should be mentioned in the release notes.

@Daniel15
Copy link
Member

Yeah @bestander was going to add some release notes for 0.17.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpojer cpojer cpojer approved these changes

@bestander bestander bestander approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@sebmck @bestander @Daniel15 @hpurmann @cpojer