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

refactor: remove common.state.tempDir #902

Closed
nfischer opened this issue Nov 8, 2018 · 0 comments
Closed

refactor: remove common.state.tempDir #902

nfischer opened this issue Nov 8, 2018 · 0 comments
Labels

Comments

@nfischer
Copy link
Member

nfischer commented Nov 8, 2018

We should remove common.state.tempDir. This really doesn't need to be in common. It makes more sense to cache this in tempdir.js itself.

nfischer added a commit that referenced this issue Nov 8, 2018
Previously, the cached `tempdir` value was stored in `common.state`.
Unlike the other `common.state` values, this isn't immediately useful to
other commands (they can just call the tempdir API). So, this moves the
cached value into `tempdir.js`.

This also adds a unit test for the caching behavior, and exposes
test-only helpers to verify this behavior.

Finally, this adds a note to `common.state` that values should generally
be considered read-only, since this can be important for customized
behavior. Although, I recognize our code base has one exception to this
rule (`echo()`), we should strive to maintain this.

Fixes #902
Test: Added a unit test.
nfischer added a commit that referenced this issue Nov 9, 2018
Previously, the cached `tempdir` value was stored in `common.state`.
Unlike the other `common.state` values, this isn't immediately useful to
other commands (they can just call the tempdir API). So, this moves the
cached value into `tempdir.js`.

This also adds a unit test for the caching behavior, and exposes
test-only helpers to verify this behavior.

Finally, this adds a note to `common.state` that values should generally
be considered read-only, since this can be important for customized
behavior. Although, I recognize our code base has one exception to this
rule (`echo()`), we should strive to maintain this.

Fixes #902
Test: Added a unit test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant