-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix wrong added filename #4919
Fix wrong added filename #4919
Conversation
I am wondering why no tests have detected it. |
License: MIT Signed-off-by: Masahiro Saito <camelmasa@gmail.com>
@Kubuxu Thanks for checking. I just changed test data directory path from Case of original code, Returned value was changed after I just change the directory path. |
@@ -336,7 +337,10 @@ func AddR(n *core.IpfsNode, root string) (key string, err error) { | |||
return "", err | |||
} | |||
|
|||
f, err := files.NewSerialFile(root, root, false, stat) | |||
segs := strings.Split(root, "/") | |||
name := segs[len(segs)-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use name := filepath.Base(root)
instead of string splitting.
Filepath is aware of system specific separators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably broke go test
on windows - https://ci.ipfs.team/blue/organizations/jenkins/IPFS%2Fgo-ipfs/detail/PR-4919/4/pipeline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One system compatibility replated change.
Since the author has been gone for ~1mo, can someone else take over this changeset? cc @djdv maybe? |
Amended in: #5167 |
I think It looks
files.NewSerialFile
's first argument is wrong.Note: The
coreunix.AddR
will be removed after implemented Core API (Ref: #4562).