Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Review Flux code duplication #4

Closed
dholbach opened this issue Sep 4, 2019 · 5 comments · Fixed by #8
Closed

Review Flux code duplication #4

dholbach opened this issue Sep 4, 2019 · 5 comments · Fixed by #8

Comments

@dholbach
Copy link
Contributor

dholbach commented Sep 4, 2019

@stefanprodan pointed out that some code of Flux made its way into gitops-toolkit. Could this be made a dependency instead?

[daniel@reef ~ ]$ ls ~/dev/{flux,gitops-toolkit/pkg}/git 
/home/daniel/dev/flux/git:
errors.go  export.go  export_test.go  gittest  mirrors.go  operations.go  operations_test.go  repo.go  signature.go  url.go  url_test.go  working.go

/home/daniel/dev/gitops-toolkit/pkg/git:
errors  errors.go  export.go  gitdir  mirrors.go  operations.go  repo.go  signature.go  url.go  working.go
[daniel@reef ~ ]$ diff -ruN ~/dev/{flux,gitops-toolkit/pkg}/git | diffstat
 errors.go            |   23 +-
 errors/errors.go     |   95 +++++++++++
 export.go            |   12 -
 export_test.go       |   44 -----
 gitdir/gitdir.go     |  234 ++++++++++++++++++++++++++++
 gittest/repo.go      |  122 --------------
 gittest/repo_test.go |  270 --------------------------------
 operations.go        |   46 ++---
 operations_test.go   |  424 ---------------------------------------------------
 repo.go              |   51 +-----
 url.go               |    2 
 url_test.go          |   20 --
 working.go           |   94 +++++++----
 13 files changed, 434 insertions(+), 1003 deletions(-)
[daniel@reef ~ ]$ 
@dholbach
Copy link
Contributor Author

dholbach commented Sep 4, 2019

Ok, maybe it's backwards and Flux could be using gitops-toolkit at some stage?

@dholbach
Copy link
Contributor Author

dholbach commented Sep 4, 2019

At least for the time being, we should attribute the code and make sure it's obvious where it came from.

@squaremo
Copy link

squaremo commented Sep 4, 2019

Ok, maybe it's backwards and Flux could be using gitops-toolkit at some stage?

The code is (C) the Flux authors.

@luxas
Copy link
Contributor

luxas commented Sep 5, 2019

So we're happy to vendor the flux code as-is as long as we can fix the TODOs that are there in the code, today this code have known issues with different edge cases for general, non-flux usage. The fork can/hopefully is temporary if flux can merge the patches that are needed.

After the temporary fork/vendor of the code, upstream has changed, hence the diff above.

cc @stealthybox

@luxas
Copy link
Contributor

luxas commented Sep 5, 2019

The thing here is that we need a generic git library in the gitops toolkit, and what flux had worked almost, but not without some patching. One (possibly) good way to do this would be to use a "real" generic git library like https://godoc.org/gopkg.in/src-d/go-git.v4 instead

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants