-
-
Notifications
You must be signed in to change notification settings - Fork 408
build_cache.path correction #2921
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -62,9 +62,11 @@ configuration file. | |||||
|
||||||
- on Linux (and other Unix-based OS) is: if | ||||||
[`$XDG_CACHE_HOME`](https://specifications.freedesktop.org/basedir-spec/latest/#variables) is defined, | ||||||
`$XDG_CACHE_HOME/arduino`. Otherwise `{HOME}/.config/arduino`. | ||||||
- on Windows is: `{HOME}/AppData/Local/arduino` | ||||||
- on MacOS is: `{HOME}/Library/Caches/arduino` | ||||||
`$XDG_CACHE_HOME/arduino`. Otherwise `$HOME/.cache/arduino`. | ||||||
- on Windows is: `%LocalAppData%/arduino` | ||||||
- on MacOS is: `$HOME/Library/Caches/arduino` | ||||||
|
||||||
If neither `$HOME` nor `$XDG_CACHE_HOME` are defined, a temporary directory will be used. | ||||||
Comment on lines
+68
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This is an emergency "belt and suspenders" fallback measure. We don't need to clutter up the documentation with information that will only be relevant for the <0.01% of users who have a completely borked operating system. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems more evil than reasonable by hiding vital implementation detail :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My decision stands. If you don't remove this then the pull request will be rejected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not belt and suspenders, if we are explaining in depth how the system works for the specific reason to avoid misunderstanding it. Consider container with incorrectly configured env, or incorrectly sandboxed shell session Or a 2nd opinion, perhaps? I am truly misunderstanding the reasoning here, and the harsh response. |
||||||
|
||||||
- The `directories.data` default is OS-dependent: | ||||||
|
||||||
|
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.
This is the canonical capitalization:
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.
True, but Windows is case insensitive here.
It is more readable that just all upper or lower case though?
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.
If we are going down the path of discussing which of arbitrary forms of the variable name are most clear, I will argue that all caps most effectively communicates that we are referring to an environment variable, as that is the standard convention.