Skip to content

Conversation

@alilleybrinker
Copy link
Member

This is intended to make the API clearer in separating the hash, which is part of the GitOid but not sufficient for representing the GitOid, from the overall GitOid structure. The GitOid can be represented as a URI, or you can extract the parts (hash algorithm, object type, hash) individually. If you have the Hash, now represented as a struct, you can either access the individual bytes through the Deref impl or as_bytes method, or you can get the hexadecimal encoding through the Display impl or the implied to_string method or explicit hex method.

Signed-off-by: Andrew Lilley Brinker alilleybrinker@gmail.com

This is intended to make the API clearer in separating the hash,
which is part of the `GitOid` but not sufficient for representing
the `GitOid`, from the overall `GitOid` structure. The `GitOid`
can be represented as a URI, or you can extract the parts (hash
algorithm, object type, hash) individually. If you have the `Hash`,
now represented as a struct, you can either access the individual
bytes through the `Deref` impl or `as_bytes` method, or you can
get the hexadecimal encoding through the `Display` impl or the
implied `to_string` method or explicit `hex` method.

Signed-off-by: Andrew Lilley Brinker <alilleybrinker@gmail.com>
@alilleybrinker
Copy link
Member Author

One thing I am definitely open for feedback on is the name of the new struct. The standard library has std::hash::Hash, which is a trait for hash-able types. The struct here might instead be named HashRef or something similar to differentiate it.

@alilleybrinker alilleybrinker added type: feature New feature or request crate: gitoid Relating to the gitoid crate labels Sep 20, 2022
This commit derives some common traits for the `Hash` struct,
to make it more usable/complete in terms of providing the
normally-expected APIs.

Signed-off-by: Andrew Lilley Brinker <alilleybrinker@gmail.com>
@nellshamrell
Copy link
Collaborator

I agree with the need to differentiate the new struct from std::hash::Hash and I think HashRef is a reasonable name for it.

@nellshamrell
Copy link
Collaborator

Other than my comment ^, this looks good!

This commit renames the `Hash` struct to `HashRef` to avoid any
conflicts with the `std::hash::Hash` trait.

Signed-off-by: Andrew Lilley Brinker <alilleybrinker@gmail.com>
@alilleybrinker
Copy link
Member Author

Great! I've done the rename and changed the import of use std::hash::Hash as StdHash in gitoid.rs to a more normal use std::hash::Hash.

Copy link
Collaborator

@nellshamrell nellshamrell left a comment

Choose a reason for hiding this comment

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

Looks great! Merging!

@nellshamrell nellshamrell merged commit b805e2a into omnibor:main Sep 22, 2022
@alilleybrinker alilleybrinker deleted the feat/hash_struct branch September 22, 2022 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crate: gitoid Relating to the gitoid crate type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants