Skip to content

Conversation

lunny
Copy link
Member

@lunny lunny commented Sep 2, 2025

Now the wikipath will not be referenced directly.

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Sep 2, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 2, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Sep 2, 2025
@lunny lunny marked this pull request as ready for review September 3, 2025 02:27
@lunny lunny changed the title Use gitrepo.Repository instead of most of wikipath Use gitrepo.Repository instead of wikipath Sep 17, 2025
@lunny lunny added this to the 1.26.0 milestone Sep 25, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 8, 2025
@yp05327
Copy link
Contributor

yp05327 commented Oct 10, 2025

It seems that the tests are removed, but there's no new one added?

@lunny
Copy link
Member Author

lunny commented Oct 10, 2025

It seems that the tests are removed, but there's no new one added?

The method repo_model.WikiPath has been removed, so that the related test is removed as well.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 15, 2025
Comment on lines 12 to 18
func CloneIn(ctx context.Context, dstRepo Repository, from string, opts git.CloneRepoOptions) error {
return git.Clone(ctx, from, repoPath(dstRepo), opts)
}

func CloneOut(ctx context.Context, fromRepo Repository, to string, opts git.CloneRepoOptions) error {
return git.Clone(ctx, repoPath(fromRepo), to, opts)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

CloneRepoToLocal

CloneLocalToRepo, but I don't understand why it is needed, why a git repo should be cloned from local to the managed repo directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

CloneIn means clone an external repository into the managed repository. like migration from github or other system. CloneOut means CloneRepoToLocal

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

But why you flip the argument order?

git clone from to, git.Clone(from, to), why here you sometimes use to, from, sometimes use from, to order?

Can there be some consistence?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 1153e9f

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

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants