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: Registry cache for Azure DevOps #1690

Closed
wants to merge 4 commits into from
Closed

fix: Registry cache for Azure DevOps #1690

wants to merge 4 commits into from

Conversation

diznq
Copy link

@diznq diznq commented Mar 29, 2023

Summary

Go Git doesn't work with Azure DevOps due to unsupported protocol: go-git/go-git#64

Output

If dev.azure.com/ URL is used for Git, git clone is used instead of Go Git library, the downside is git must be installed locally

Signed-off-by: Jakub <13458283+diznq@users.noreply.github.com>
@diznq diznq requested review from a team as code owners March 29, 2023 07:47
@github-actions github-actions bot added this to the 0.30.0 milestone Mar 29, 2023
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Mar 29, 2023
@jkutner
Copy link
Member

jkutner commented Mar 29, 2023

I think it's ok to put a dependency on git. We just need to make sure we fail in a way that makes it clear. We might need a better error message

URL: r.url.String(),
})
if err != nil {
return errors.Wrap(err, "cloning remote registry")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could such error handling have been more perfect?

diznq and others added 2 commits March 30, 2023 08:20
Co-authored-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Jakub <13458283+diznq@users.noreply.github.com>
Co-authored-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Jakub <13458283+diznq@users.noreply.github.com>
@cubxxw
Copy link
Contributor

cubxxw commented Mar 30, 2023

Strange, did you make mistakes with the rebase, why are there multiple commits?

Signed-off-by: Jakub <13458283+diznq@users.noreply.github.com>
@diznq
Copy link
Author

diznq commented Mar 30, 2023

I just signed off the comments raised here but realized strings import wasn't removed and Go is strict about unused stuff

@cubxxw
Copy link
Contributor

cubxxw commented Mar 30, 2023

This indicates that recent changes have not achieved the desired coverage, and more work needs to be done to increase coverage.

@diznq diznq closed this Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants