Skip to content

Add TempDir#62

Merged
mcuadros merged 1 commit into
src-d:masterfrom
kuba--:enhancement-tempdir
Sep 19, 2018
Merged

Add TempDir#62
mcuadros merged 1 commit into
src-d:masterfrom
kuba--:enhancement-tempdir

Conversation

@kuba--

@kuba-- kuba-- commented Aug 13, 2018

Copy link
Copy Markdown

Signed-off-by: kuba-- kuba@sourced.tech

Closes: #54

@kuba-- kuba-- requested review from a team and smola August 13, 2018 19:37
Comment thread fs.go Outdated
type Filesystem interface {
Basic
TempFile
TempDir

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding TempDir to the interface will force a major upgrade, I fill that doesn't worth to do it.
Maybe in the future with more updates.

@smola

smola commented Aug 14, 2018

Copy link
Copy Markdown
Contributor

Can't we implement it as an utility function, just as util.TempFile, using the existing interfaces?

@kuba--

kuba-- commented Aug 14, 2018

Copy link
Copy Markdown
Author

@smola - util.TempDir is already there. Either we can keep it in utils and remove signature from interfaces or we can close this PR.

@smola

smola commented Aug 14, 2018

Copy link
Copy Markdown
Contributor

I think 👍 to keeping it in utils without interface changes. Utility functions that are present in the standard library are usually nice to have in billy.

@kuba--

kuba-- commented Aug 14, 2018

Copy link
Copy Markdown
Author

Ok, I left only implementation in utils.

Comment thread .gitignore
@@ -1 +1,4 @@
/coverage.txt
/vendor

@ajnavarro ajnavarro Aug 14, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we remove that lines from .gitignore?. go-billy is not using dep right now.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

that's why I added ;) to avoid a mistake and commit vendor dir

@kuba--

kuba-- commented Aug 17, 2018

Copy link
Copy Markdown
Author

shall we close it or merge it?

@smola

smola commented Aug 21, 2018

Copy link
Copy Markdown
Contributor

ping @mcuadros

Comment thread util/util.go
// Multiple programs calling TempDir simultaneously
// will not choose the same directory. It is the caller's responsibility
// to remove the directory when no longer needed.
func TempDir(fs billy.Dir, dir, prefix string) (name string, err error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not covered by any test

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@mcuadros - I've added 2 tests for TempDir and TempFile.

Signed-off-by: kuba-- <kuba@sourced.tech>
@mcuadros mcuadros merged commit 9826264 into src-d:master Sep 19, 2018
@kuba-- kuba-- deleted the enhancement-tempdir branch September 19, 2018 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants