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

Add Tree#find_blob. Account for symlink blobs in Tree class. #59

Merged
merged 3 commits into from
Mar 28, 2023

Conversation

dometto
Copy link
Member

@dometto dometto commented Jan 15, 2023

Implements the specs proposed in gollum/adapter_specs#16

@dometto dometto merged commit 68b083c into gollum:master Mar 28, 2023
@dometto dometto deleted the improve_tree_blobs branch March 28, 2023 15:27
@@ -689,6 +685,16 @@ def find_branch(search_list)

class Tree

def self.tree_entry_from_rugged_hash(entry, root = '')
Copy link
Member

Choose a reason for hiding this comment

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

I'm not loving this name of this class method. One reason is that the name suggests that the method returns a tree entry based on hash input ("from hash"), but the name of the first argument and the body of the method imply that an entry (whatever that is) is transformed into a hash (thus the other way around).

A second reason is that I have no sense of what a rugged_hash is. It seems as if is is an ordinary hash originating from a method in rugged, but then I don't see why we need to know the origin of the hash.

Would it make sense to rename this to something like tree_entry_to_hash(entry, root='')?

Copy link
Member

Choose a reason for hiding this comment

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

(Sorry, I know this is merged already.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good points, thanks! The wording is tricky since a tree entry is essentially just a hash representation of an object in a git tree, as used by rugged, and we want to convert this into our own hash representation of an object in a git tree. tree_entry_to_hash misleadingly sounds as if what we're passing in is a hash instead of a tree entry. What about something like canonicalize_tree_entry?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants