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

fix(clone): reset index of YADM_WORK #507

Merged
merged 5 commits into from
Nov 29, 2024

Conversation

AVM-Martin
Copy link

What does this PR do?

Re-open PR #492

Whenever using clone command inside YADM_WORK sub-directory, we need
to checkout the correct repo contents.

Steps to reproduce:

mkdir $HOME/subdir
cd $HOME/subdir

yadm clone --bootstrap "<repo-url>"
yadm status

What issues does this PR fix or reference?

[A list of related issues / pull requests.]

Previous Behavior

yadm status reported a lot of deleted files.

New Behavior

yadm status will report clean working directory.

Have tests been written for this change?

Yes

Have these commits been signed with GnuPG?

Yes


Please review yadm's Contributing Guide for best practices.

Whenever using `clone` command inside YADM_WORK sub-directory, we need
to checkout the correct repo contents.

Steps to reproduce:

```bash
mkdir $HOME/subdir
cd $HOME/subdir

yadm clone --bootstrap "<repo-url>"
yadm status
```
Copy link
Collaborator

@erijo erijo left a comment

Choose a reason for hiding this comment

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

Thanks for you PR. Just a few minor comments that would be good if you could address.

Comment on lines 346 to 347
args = ["clone", "-w", paths.work]
args += [remote_url]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please merge these two lines into one.

Copy link
Author

Choose a reason for hiding this comment

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

Done in a7939be

# clone should succeed, and repo should be configured properly
assert successful_clone(run, paths, repo_config)

# test that the conflicts are preserved in the work tree
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like an incorrect comment?!

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in 85e8c1d, english is not my first language

yadm Outdated
@@ -827,7 +827,7 @@ function clone() {
rm -rf "$wc"

# then reset the index as the --no-checkout flag makes the index empty
"$GIT_PROGRAM" reset --quiet -- .
"$GIT_PROGRAM" reset --quiet -- "$YADM_WORK"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider using :/ instead of $YADM_WORK to match the pathspec used below (in the checkout command).

Copy link
Author

Choose a reason for hiding this comment

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

Nice info, I just know about pathspec
Changed in ae3a149

@erijo erijo changed the base branch from master to develop November 29, 2024 22:10
@erijo erijo merged commit 6ee9b47 into yadm-dev:develop Nov 29, 2024
1 check passed
@erijo
Copy link
Collaborator

erijo commented Nov 29, 2024

Thanks for your contribution @AVM-Martin! 👍

@AVM-Martin AVM-Martin deleted the fix/reset-yadm-work-index branch December 1, 2024 07:10
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