Closed ollie-kane-CB closed 1 month ago
ollie-kane-CB
commented
1 month ago I see the tests have failed. I'm a bit unfamiliar with Go, but if the effort is needed, i can attempt to figure out what's wrong. I expected this to be the equiliv of adding the --depth 1 flag to a git clone
git clone --depth 1 REPO_URL .rather than
git clone REPO_URL .After reading on the web some, it seems like this link may have something to do with it.
https://github.com/rancher/rancher/issues/42659
They suggest that in addition to the Depth, one must also specify the Force param, otherwise it won't create the local ref. I'll attempt this and see if it passes the test suite. If not, rather than generate a lot of noise, I'll see if I can determine how to run your test suite myself.
ollie-kane-CB
commented
1 month ago Modified this to Draft. I figured out how to build and run the project locally. Will iterate and return.
ollie-kane-CB
commented
1 month ago Updated this to pass the unit tests. Ended up being slightly more complicated than I expected.
In order to support the Depth: 1, the Hash resolver process needed to be updated. Instead of resolving the gitRevision using the results of a Fetch --all, instead use the GitRemove.List operation to get a list of refs only, and from those resolve the hash. Once the hash is known, use that in the FetchOptions to configure only fetching of the data required.
A small side effect is the 'doesnotexist' test needed to have it's error message updated as the error thrown when there's no match is different now.
Sorry for the delay and noise here!
codecov-commenter
commented
1 month ago :warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Attention: Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
Project coverage is 69.82%. Comparing base (
49014e2) to head (f8b073d). Report is 51 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| pkg/git/fetch.go | 86.66% | 1 Missing and 1 partial :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
ollie-kane-CB
commented
1 month ago lgtm
Thanks for the merge!
I'm looking into adopting this project into my stack. I don't know if I'll ever get around to it, but if you were to pick a feature/bug/issue that'd be the best value-add, what would that be? If I'm swimming in this lane, it's possible that a weekend here i may take a stab at the backlog.
tomkennedy513
commented
1 month ago lgtm
Thanks for the merge!
I'm looking into adopting this project into my stack. I don't know if I'll ever get around to it, but if you were to pick a feature/bug/issue that'd be the best value-add, what would that be? If I'm swimming in this lane, it's possible that a weekend here i may take a stab at the backlog.
hmm, I'm not sure I have a particular issue I would recommend, but I would say that playing around with kpack and seeing what doesn't work right or what annoys you that could be improved might be a good way to start
For repositories with large histories, since there doesn't appear to be caching of the git repository, we shouldn't pull all history as it's not needed for the purpose of building. This one line change significantly improves build performance when irrelevant git history is large.