Skip to content

git: checkout.validate should set to branch only if both branch and hash are empty #1507

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

git: checkout.validate should set to branch only if both branch and hash are empty #1507

wants to merge 10 commits into from

Conversation

GaikwadPratik
Copy link

This would avoid the error condition where if Hash is provided on CheckoutOptions but NOT branch and validate is called since per documentation:

// Branch to be checked out, if Branch and Hash are empty is set to master.
Branch plumbing.ReferenceName

@pjbgf
Copy link
Member

pjbgf commented Apr 13, 2025

@GaikwadPratik thanks for looking into this. Please add some tests to avoid future regressions.

@GaikwadPratik
Copy link
Author

Hi @pjbgf, added couple of unit tests and comments in the code for easy understanding. Can you please re-review?

dependabot bot and others added 7 commits April 13, 2025 21:47
@dependabot @GaikwadPratik
build: bump github.com/ProtonMail/go-crypto from 1.1.6 to 1.2.0
1426c81 
Bumps [github.com/ProtonMail/go-crypto](https://github.com/ProtonMail/go-crypto) from 1.1.6 to 1.2.0.
- [Release notes](https://github.com/ProtonMail/go-crypto/releases)
- [Commits](ProtonMail/go-crypto@v1.1.6...v1.2.0)

---
updated-dependencies:
- dependency-name: github.com/ProtonMail/go-crypto
  dependency-version: 1.2.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@patricsss @GaikwadPratik
utils: fix diff so subpaths work for sparse checkouts. Fixes #1455
6f922af
@0xDEC0DE @GaikwadPratik
fix: correct matching of file vs. dir globs
2b2967c 
e.g., the pattern `/a*/**` in `.gitignore` should not match a file
`ab` in the project root.

Fix a broken test that was enforcing the incorrect behavior.

Fixes: Issue #154
@PascalBourdier @GaikwadPratik
plumbing: fix err mngt in packfile processDelta
a0c994b 
this func could fail silently and couldn't be catch at higher level
@PascalBourdier @GaikwadPratik
plumbing: blame simplification
2163007 
remove unused result parameter and dead code
@GaikwadPratik
git: checkout.validate should set to branch only if both branch and h…
d845e29 
…ash are empty
@GaikwadPratik
git: add unit tests for checkout options
f8c20f6
@pjbgf pjbgf changed the title fix: checkout.validate should set to branch only if both branch and hash are empty git: checkout.validate should set to branch only if both branch and hash are empty Apr 15, 2025
pjbgf
pjbgf requested changes Apr 15, 2025
View reviewed changes
Copy link
Member

@pjbgf pjbgf left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@GaikwadPratik please squash your commits and name them in line with the contributing guidelines.

You can either have a single commit, or split it into two commits, one for the Force/Keep and once for the Branch/Hash check.

@GaikwadPratik
Copy link
Author

This appears to be rebasing of the fork... I'll open a new PR and close this so history stays clean

@pjbgf
Copy link
Member

pjbgf commented Apr 18, 2025

@GaikwadPratik that's absolutely fine. For future reference, you can rebase with:

git fetch upstream
git rebase upstream/<branch>

When the branch gets a bit messy, you can always reset it (taking note of the commits you are interested in first), then cherry picking your commits:

git fetch upstream
git reset upstream/<branch> --hard
git cherry-pick <commit1> <commit2>

However, feel free to just close the PR and create a new one if that's more convenient.

@GaikwadPratik GaikwadPratik closed this by deleting the head repository Apr 19, 2025
@GaikwadPratik
Copy link
Author

Thanks @pjbgf for suggstions. but I ended up creating new PR #1525. Since the git history was messed up a lot due to rebase of the branch already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjbgf pjbgf pjbgf requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@GaikwadPratik @pjbgf @patricsss @0xDEC0DE @PascalBourdier