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

resolve homedir #106

Merged
merged 1 commit into from
Dec 8, 2020
Merged

resolve homedir #106

merged 1 commit into from
Dec 8, 2020

Conversation

aschmahmann
Copy link
Contributor

@aschmahmann aschmahmann commented Jun 19, 2020

While go-ipfs will resolve ~/myipfspath into userHomeDir/myipfspath fs-repo-migrations would not. This PR adds resolving ~ into the users' home directory.

Review by commit, since this PR includes some overdue go fmt changes as well.

@aschmahmann aschmahmann requested a review from Stebalien June 19, 2020 18:13
func encodeBlock(dst, src []byte) (d int)
Copy link
Member

Choose a reason for hiding this comment

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

Ah, these are vendored deps. We should just leave them alone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so just remove the go fmt commit? Can do, although it means go fmt ./... is always going to cause problems in this repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. Unfortunately, our vendoring game here could use some work.

main.go Outdated
return ipfspath, nil
expandedPath, err := homedir.Expand(ipfspath)
if err != nil {
return "", nil
Copy link
Member

Choose a reason for hiding this comment

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

no error? This seems like an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. Although question, is it safe/smart to be using homedir this vendored way homedir "github.com/ipfs/fs-repo-migrations/ipfs-2-to-3/Godeps/_workspace/src/github.com/mitchellh/go-homedir"?

Copy link
Member

Choose a reason for hiding this comment

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

It should be fine.

@aschmahmann aschmahmann merged commit c3f148e into master Dec 8, 2020
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.

2 participants