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

libgit2: add remaining checkout strategy tests #458

Merged
merged 2 commits into from
Oct 22, 2021
Merged

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Oct 22, 2021

This PR is a follow up on #457,
and adds tests for the remaining checkout strategies, while
consolidating some of the logic.

The consolidated logic ensures that (SemVer) tag and commit checkouts
happen using the same "checkout detached HEAD" logic.
The branch checkout is left unmodified, and simply checks out at the
current HEAD of the given branch.

In addition, it handles the Free of most objects and thereby
supersedes #452.

@hiddeco hiddeco changed the title Simplify checkout libgit2: add remaining checkout strategy tests Oct 22, 2021
@hiddeco hiddeco added the area/git Git related issues and pull requests label Oct 22, 2021
Copy link
Member

@stefanprodan stefanprodan 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 great! I think it would be good to document in the PR body that we are switching to detached HEAD for all checkout strategies.

This commit is a follow up on 4dc3185
and adds tests for the remaining checkout strategies, while
consolidating some of the logic.

The consolidated logic ensures that (SemVer) tag and commit checkouts
happen using the same "checkout detached HEAD" logic.
The branch checkout is left unmodified, and simply checks out at the
current HEAD of the given branch.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit ensures most of the `git2go` objects `Free` themselves from
the underlying C object.

Ensuring all objects are freed is not possible yet, due to the way
commits are wired in to facilitate verification later on. In a later
follow up, we should change this and e.g. validate as part of the
checkout process, and move the implementation specific authentication
configuration from `git` into `libgit2`.

Signed-off-by: Hidde Beydals <hello@hidde.co>
@hiddeco hiddeco marked this pull request as ready for review October 22, 2021 10:02
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @hiddeco 🏅

@hiddeco hiddeco merged commit b35d7d8 into main Oct 22, 2021
@hiddeco hiddeco deleted the libgit2-simple-tag branch October 22, 2021 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants