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

adding fileutil #38

Merged
merged 6 commits into from Apr 27, 2022
Merged

adding fileutil #38

merged 6 commits into from Apr 27, 2022

Conversation

tvoran
Copy link
Member

@tvoran tvoran commented Apr 14, 2022

Adds a fileutil package, mainly for the purpose of sharing the caching file reader from https://github.com/hashicorp/vault-plugin-auth-kubernetes/blob/main/caching_file_reader.go

for sharing caching file reader
tomhjp
tomhjp previously approved these changes Apr 14, 2022
fileutil/caching_file_reader.go Outdated Show resolved Hide resolved
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
tomhjp
tomhjp previously approved these changes Apr 14, 2022
swenson
swenson previously approved these changes Apr 14, 2022
Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM; small nits about removing ioutil

fileutil/caching_file_reader.go Outdated Show resolved Hide resolved
fileutil/caching_file_reader_test.go Outdated Show resolved Hide resolved
@tvoran tvoran dismissed stale reviews from swenson and tomhjp via a3f849d April 14, 2022 21:49
@tvoran tvoran requested review from tomhjp and swenson April 15, 2022 23:19
and protect time updates in the test
Use []byte instead of string to store the contents of the file, and
always return a copy from ReadFile().
@calvn calvn merged commit 17e9a8f into main Apr 27, 2022
@calvn calvn deleted the VAULT-5669/caching-file-reader branch April 27, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants