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

unixfs: Use only one kind of variable to refer to data types #5055

Closed
schomatis opened this issue May 31, 2018 · 3 comments
Closed

unixfs: Use only one kind of variable to refer to data types #5055

schomatis opened this issue May 31, 2018 · 3 comments
Assignees
Labels
topic/files Topic files topic/technical debt Topic technical debt

Comments

@schomatis
Copy link
Contributor

There are a series of alias for the protocol buffer types of the Data message (format), in some parts of the code the alias are used (e.g., TFile) while in others the original variables are (e.g., pb.Data_File) which makes it harder to follow the code logic, these should be unified and only one kind of variable should be used.

https://github.com/ipfs/go-ipfs/blob/e235d02188260a7a3df18209df1f11b6bf561483/unixfs/unixfs.go#L15-L19

https://github.com/ipfs/go-ipfs/blob/e235d02188260a7a3df18209df1f11b6bf561483/importer/helpers/dagbuilder.go#L119

https://github.com/ipfs/go-ipfs/blob/e235d02188260a7a3df18209df1f11b6bf561483/mfs/dir.go#L168

(Also the unixfs and the unixfs/pb have several different import alias which makes it even harder to quickly infer from the code what are the types referring to without having to look at the import statements.)

I would be inclined to remove the alias (to eliminate the possibitlity of this double reference) and simplyfiy names at the protocol buffer level in the .proto file (if possible, that will be discussed in a separate issue).

@schomatis schomatis added topic/files Topic files topic/technical debt Topic technical debt labels May 31, 2018
@schomatis schomatis added this to the Files API Documentation milestone May 31, 2018
@schomatis schomatis self-assigned this May 31, 2018
@schomatis
Copy link
Contributor Author

At this point I don't think we can rename the proto-buf names, so let's try to use the unixfs short alias as much as possible throughout the code base (and thus avoid direct reference to the unixfs_pb package -inunixfs.pb.go- as much as possible as we've been doing with ipfs/go-unixfs#17).

/cc @overbool

@overbool
Copy link
Contributor

overbool commented Sep 26, 2018

@schomatis Did you mean that we should use data type under unixfs package instead of data type under unixfs_pb package as much as possible throughout the code base , eg, using unixfs.TFile instead of pb.Data_File?

@schomatis
Copy link
Contributor Author

Yes exactly!

overbool added a commit to overbool/go-ipfs that referenced this issue Sep 26, 2018
License: MIT
Signed-off-by: Overbool <overbool.xu@gmail.com>
overbool added a commit to overbool/go-ipfs that referenced this issue Sep 26, 2018
License: MIT
Signed-off-by: Overbool <overbool.xu@gmail.com>
Stebalien added a commit that referenced this issue Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/files Topic files topic/technical debt Topic technical debt
Projects
None yet
Development

No branches or pull requests

2 participants