Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

Add gitHead in subdirectories too #80

Closed
wants to merge 1 commit into from

Conversation

felixfbecker
Copy link
Contributor

Fixes #66

@felixfbecker
Copy link
Contributor Author

CI failure is because latest npm is installed on Node 4

@felixfbecker
Copy link
Contributor Author

@zkat @iarna could one of you give this a review? :)

@felixfbecker
Copy link
Contributor Author

Friendly ping? This would really help a bunch of tools in the npm ecosystem

return cb(null, data)
}
return githead(dir, data, cb)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be githead_?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so - githead() reads the .git/HEAD file (if found) and passes the content to githead_(). So to traverse up the tree and find the .git/HEAD file, we need to call githead() recursively.
githead_() is what then handles the contents of the .git/HEAD file by reading the ref it points to etc.

It's easy to get confused though with the naming of the functions and no comments...

Copy link
Contributor

@iarna iarna left a comment

Choose a reason for hiding this comment

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

This looks good to me (but note: I'm not a committer here now) and per the RFC meeting either it will get merged or something that does the same thing will be. Personally I'd like to see this get merged as is to get the feature out there and in the ecosystem as early as possible.

@wraithgar
Copy link
Member

Thanks for your patience here. I'm getting this repo's CI up to date and then we can rebase, relint, retest, and ship it.

Note that gitHead.js was renamed because case sensitive filenames are something we are trying to avoid.

@wraithgar
Copy link
Member

#101

@wraithgar wraithgar closed this Aug 23, 2021
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.

gitHead not populated if we operate out of a sub directory
5 participants