-
Notifications
You must be signed in to change notification settings - Fork 621
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
Enhance commit retrieval with branch & tag prefix support #3518
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I've added a couple of comments re error handling. Otherwise, it looks good to me 👍🏻
ffe2521
to
1c36f1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
bf7d9b8
to
dd0342a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refFormats := []string{ | ||
ref, // Try as a commit hash | ||
"heads/" + ref, // Try as a branch | ||
"tags/" + ref, // Try as a tag | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually I think we will want to try be smarter about using the SHA, heads, or tags ref. Someone could theoretically have the same name for a branch and tag.
I don't think we should address this here, as this PR is enough of an improvement already, but it is something to think of for the future.
GitHub's API documentation states:
This PR enhances our commit retrieval to align with this specification:
heads/
,tags/
) when fetching commitsgitHubCommitGetter
interface for better abstraction