feat: add helper to retrieve a Repository URL from a given path #18
Conversation
… into feat/CLI-195_repourl
|
|
PeterSchafer
force-pushed
the
feat/CLI-195_repourl
branch
from
eb2fbff to
6283e6d
Compare
| go 1.21 | ||
|
|
||
| require ( | ||
| github.com/go-git/go-git/v5 v5.11.0 |
There was a problem hiding this comment.
Going to play a bit of devil's advocate here... what if we parsed the [remote "origin"] section out of .git/config directly, instead of bringing in this dependency? I think it's a pretty well-defined and simple format.
Another thing to consider, we might only need the config parser (https://pkg.go.dev/github.com/go-git/go-git/v5@v5.11.0/config#ReadConfig) out of go-git.
There was a problem hiding this comment.
Good question! In this case, I was just following the implementation hints in the ticket.
- We are using the library in the tests as well, not only for reading the config but also for cloning a test repo.
- How big is our appetite in avoiding this dependency vs implementing this our own?
There was a problem hiding this comment.
For what it's worth, the reason the IDE team suggested go-git is because we will probably end up usong it anyway for https://snyksec.atlassian.net/wiki/spaces/RD/pages/1842544646/IDE+Only+display+delta+findings. So even if we don't add it to code-client-go, we will add it to snyk-ls. Of course, this means the dependency will still be added to the CLI but just an fyi
There was a problem hiding this comment.
Does the code client need git access? Or could/should we pass in an interface here (similar to how we pass in config) and leave it to snyk-ls to implement with go-git?
There was a problem hiding this comment.
It doesn't but I think it would be better to centralise how we compute the git repo URL so it's not different across the two, which is why it makes sense to do that computation here. What's the reason for not including go-git? If it's a big concern then we could do what you suggested and not use go-git but I prefer letting a library deal with possible edge cases if I can 🤷
There was a problem hiding this comment.
I think Casey meant that we could define an interface based on go-git exported functions, that supports only the operations we need in code-client and that is automatically then implemented by go-git if a go-git type instance is passed in from snyk-ls. This would decouple the client a bit more, although I'm fine with using a 3rd-party library in this case. And using the dependency directly makes the code easier to maintain and navigate, as an indirection layer is removed.
No description provided.