Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

fix(fsnode): issue #17 #18

Merged
merged 1 commit into from
Sep 24, 2018
Merged

fix(fsnode): issue #17 #18

merged 1 commit into from
Sep 24, 2018

Conversation

overbool
Copy link
Contributor

@overbool overbool changed the title fix(fsnode): issue #17 WIP: fix(fsnode): issue #17 Sep 20, 2018
@overbool overbool changed the title WIP: fix(fsnode): issue #17 fix(fsnode): issue #17 Sep 21, 2018
@overbool
Copy link
Contributor Author

@schomatis, could u help me review it?

Copy link
Contributor

@schomatis schomatis left a comment

Choose a reason for hiding this comment

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

Great job! Could you do a quick check that this passes the go-ipfs tests? You'll need to run gx-go link go-unixfs inside the go-ipfs directory to make sure it's using your modified version.

unixfs.go Outdated
@@ -182,6 +182,21 @@ func NewFSNode(dataType pb.Data_DataType) *FSNode {
return n
}

// Filesize gets filesize of format
func (n *FSNode) Filesize() uint64 {
Copy link
Contributor

@schomatis schomatis Sep 21, 2018

Choose a reason for hiding this comment

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

This function already exists below, it can be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@schomatis I had fixed it. Thx for your reply.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@schomatis I am not sure whether we should delete FromBytes() in this pr.

Copy link
Contributor

Choose a reason for hiding this comment

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

@schomatis I am not sure whether we should delete FromBytes() in this pr.

No, it's still used in many other repos, but we should probably add a "deprecated" sign or a note recommending the use of FSNodeFromBytes.

unixfs.go Outdated
@@ -28,16 +28,6 @@ var (
ErrUnrecognizedType = errors.New("unrecognized node type")
)

// FromBytes unmarshals a byte slice as protobuf Data.
func FromBytes(data []byte) (*pb.Data, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So, let's restore this function as mentioned above and add a deprecation notice:

// Deprecated: Use `FSNodeFromBytes` instead to avoid direct manipulation of `pb.Data.format`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I had added the Deprecated description.

@Stebalien Stebalien merged commit f68092e into ipfs:master Sep 24, 2018
@schomatis schomatis mentioned this pull request Oct 15, 2018
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants