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.

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

1084 move back to go git #1159

Merged
merged 5 commits into from
Apr 3, 2023
Merged

1084 move back to go git #1159

merged 5 commits into from
Apr 3, 2023

Conversation

ncarlson
Copy link
Contributor

@ncarlson ncarlson commented Mar 6, 2023

  • replaced git2go with go-git

@ncarlson ncarlson linked an issue Mar 6, 2023 that may be closed by this pull request
Move back to go-git #1084
Closed
ncarlson
ncarlson commented Mar 6, 2023
View reviewed changes
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 6, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 6, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 6, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
ncarlson
ncarlson commented Mar 7, 2023
View reviewed changes
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2023 •
edited
Loading

Codecov Report

Merging #1159 (3dfdfff) into main (61e0ee7) will decrease coverage by 0.10%.
The diff coverage is 81.00%.

❗ Current head 3dfdfff differs from pull request most recent head 5d27789. Consider uploading reports for the commit 5d27789 to get more accurate results

@@            Coverage Diff             @@
##             main    #1159      +/-   ##
==========================================
- Coverage   70.11%   70.01%   -0.10%     
==========================================
  Files         133      132       -1     
  Lines        6719     6677      -42     
==========================================
- Hits         4711     4675      -36     
+ Misses       1578     1573       -5     
+ Partials      430      429       -1
Impacted Files Coverage Δ
pkg/git/resolver.go 0.00% <0.00%> (ø)
pkg/git/url_parser.go 100.00% <ø> (ø)
pkg/git/k8s_git_keychain.go 89.47% <66.66%> (-4.98%) ⬇️
pkg/git/fetch.go 67.14% <70.73%> (-12.27%) ⬇️
pkg/git/git_keychain.go 79.31% <90.00%> (-3.13%) ⬇️
pkg/git/remote_git_resolver.go 79.62% <100.00%> (-3.98%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ncarlson ncarlson marked this pull request as ready for review March 9, 2023 16:03
matthewmcnew
matthewmcnew reviewed Mar 14, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
matthewmcnew
matthewmcnew reviewed Mar 14, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
matthewmcnew
matthewmcnew reviewed Mar 14, 2023
View reviewed changes
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
chenbh
chenbh reviewed Mar 14, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
pkg/git/fetch.go Outdated Show resolved Hide resolved
pkg/git/fetch.go Show resolved Hide resolved
pkg/git/fetch.go Outdated Show resolved Hide resolved
pkg/git/fetch.go Outdated Show resolved Hide resolved
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
pkg/git/git_keychain.go Outdated Show resolved Hide resolved
pkg/git/k8s_git_keychain_test.go Outdated Show resolved Hide resolved
pkg/git/remote_git_resolver.go Outdated Show resolved Hide resolved
pkg/git/remote_git_resolver_test.go Outdated Show resolved Hide resolved
@ncarlson ncarlson force-pushed the 1084-move-back-to-go-git branch from 28863bf to 14b304e Compare March 21, 2023 15:20
@ncarlson ncarlson requested review from matthewmcnew and chenbh March 21, 2023 15:23
chenbh
chenbh reviewed Mar 24, 2023
View reviewed changes
pkg/git/fetch.go Outdated Show resolved Hide resolved
pkg/git/fetch_test.go Show resolved Hide resolved
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
@ncarlson ncarlson force-pushed the 1084-move-back-to-go-git branch from a05796b to 896a0f9 Compare March 27, 2023 16:42
Nicholas Carlson added 2 commits March 28, 2023 13:42
Swapping git2go for go-git
5482fb0 
- To allow for building kpack without relying on custom buildpacks to install libgit2
- Go-git now supports Azure DevOps git repos (which is why we moved from go-git to git2go previously)
Fixed failing tests and addressed PR feedback
e6bc410
@ncarlson ncarlson force-pushed the 1084-move-back-to-go-git branch from 915e852 to e6bc410 Compare March 28, 2023 19:43
chenbh
chenbh reviewed Mar 28, 2023
View reviewed changes
Copy link
Contributor

@chenbh chenbh left a comment

Choose a reason for hiding this comment

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

nit: creating (and asserting on) a boolean which hold the result of an comparison is redundant because we can just assert on the comparison itself

Also, I would get rid of the messages on the asserts. We don't use that style anywhere else in the codebase and personally, I find them useless as go tells you the exact line the assertion failed anyways.

pkg/git/fetch_test.go Outdated Show resolved Hide resolved
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
pkg/git/fetch_test.go Outdated Show resolved Hide resolved
chenbh
chenbh approved these changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@chenbh chenbh left a comment

Choose a reason for hiding this comment

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

lgtm

@ncarlson ncarlson merged commit 40d8549 into main Apr 3, 2023
@ncarlson ncarlson deleted the 1084-move-back-to-go-git branch April 3, 2023 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenbh chenbh chenbh approved these changes

@tomkennedy513 tomkennedy513 Awaiting requested review from tomkennedy513

@yaelharel yaelharel Awaiting requested review from yaelharel

@matthewmcnew matthewmcnew Awaiting requested review from matthewmcnew

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

Successfully merging this pull request may close these issues.

Move back to go-git
4 participants
@ncarlson @codecov-commenter @matthewmcnew @chenbh