pkg/ioutils: TempDir: move to pkg/longpath#44515
Merged
thaJeztah merged 4 commits intomoby:masterfrom Dec 21, 2022
Merged
Conversation
thaJeztah
added
status/2-code-review
kind/refactor
thaJeztah
force-pushed
the
move_ioutils_tempdir
branch
from
36bb4d2 to
6668c51
Compare
Member
Author
|
One failure; could be related to the first commit (which switched to use |
Member
Author
Contributor
|
Did you intend to squash the last two commits together? They have the same description |
Member
Author
Oh! Hm.. I guess I was planning to do so yes; looks like I may initially had the rename of the function separately, but then made up my mind that including it in the move itself would reduce code churn a bit. Let me squash them, and update the commit message to mention the rename. 👍 |
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
This allows us to maintain a single GoDoc string to describe what it's used for. Signed-off-by: Sebastiaan van Stijn <[email protected]>
This utility wasn't very related to all other utilities in pkg/ioutils. Moving it to longpath to also make it more clear what it does. It looks like there's only a single (public) external consumer of this utility, and only used in a test, and it's not 100% clear if it was intentional to use our package, of if it was a case of "I actually meant `io/ioutil.MkdirTemp`" so we could consider skipping the alias. While moving the package, I also renamed `TempDir` to `MkdirTemp`, which is the signature it matches in "os" from stdlib. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
move_ioutils_tempdir
branch
from
6668c51 to
c63ea32
Compare
Member
Author
|
Rebased, and squashed the last two commits 👍 |
corhere
approved these changes
Dec 21, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This utility wasn't very related to all other utilities in pkg/ioutils.
Moving it to longpath to also make it more clear what it does.
It looks like there's only a single (public) external consumer of this
utility, and only used in a test, and it's not 100% clear if it was
intentional to use our package, of if it was a case of "I actually meant
io/ioutil.MkdirTemp" (elastic/beats#16306 / https://github.com/elastic/beats/blob/26f5d5c0d4ce559b1600dcfdbea0316006ace551/libbeat/autodiscover/template/config_test.go#L335) so we could consider skipping the alias.While moving the package, I also renamed
TempDirtoMkdirTemp, whichis the signature it matches in "os" from stdlib.