search

radicle-dev / radicle-git

Everything Radicle growing around Git
Other
41 stars 5 forks source link

radicle-surf: consolidate Directory and Tree, File and Blob? #87

Closed keepsimple1 closed 1 year ago

keepsimple1 commented 1 year ago

Currently (and with this pending PR) radicle-surf has two sets of types that are related, different while partially overlapping:

  1. Directory and File: represent a directory and a file in the file system of a particular commit.
  2. Tree and Blob: represent a git tree object and a git blob object.

A Tree represents the content of a Directory, and a Blob represents the content of a File. Multiple directories can point to the same tree and multiple files can point to the same blob.

Because our users often are interested in the contents of a directory or a file, they could use either Directory, File types or Tree, Blob types to retrieve the info. In such cases, these two pairs seem to be overlapping and might cause confusions. Naturally we ask this question: should we consolidate them into only one pair of types? This issue is opened to track this question.

FintanH commented 1 year ago

I might also point out that the current Tree and Blob types would not be needed if we deprecate and remove them due to https://github.com/radicle-dev/heartwood/issues/172

cloudhead commented 1 year ago

I personally don't find it confusing, and think it might be a good idea to have both. If we should only have one type, then I'd opt for Blob/Tree, since the path is known by the user.

FintanH commented 1 year ago

I think the confusion stems from the two pairs of types being similar but having different expectations on the user end, i.e. Tree and Blob were there for serialization purposes which we're deeming as not needed in this discussion https://github.com/radicle-dev/heartwood/issues/172

cloudhead commented 1 year ago

I don't really understand how this is related to serialization. To me it's just a matter of whether we need a wrapper type around blob/tree. We have Blob { data: Vec<u8>, oid: Oid } conceptually, which is one of the basic git object types, and then the question is if we also want to have File { path: PathBuf, blob: Blob, mode: FileMode } for example.

keepsimple1 commented 1 year ago

I also don't think serialization is a major factor. It seems that our users (currently radicle-httpd) are comfortable with using Tree and Blob.

To me it's just a matter of whether we need a wrapper type around blob/tree.

Sounds good to me. I think we don't need to consolidate them. For Directory and File, we can keep them and their API would be clearer once people start to utilize them.

FintanH commented 1 year ago

It was related to serialization because that was the marked difference between the two types, originally. Tree existed to be Serialize for the http-api/Upstream code, similarly for Blob :) But it seems like we've come to a good conclusion :+1:

FintanH commented 1 year ago

Sounds good to me. I think we don't need to consolidate them. For Directory and File, we can keep them and their API would be clearer once people start to utilize them.

I'll close this issue then :)