-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: add logger interface #17
Conversation
Preserve oid batch arguments in requset/response per [specs](https://github.com/git-lfs/git-lfs/blob/main/docs/proposals/ssh_adapter.md#requests-to-transfer-objects)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely an improvement. If you haven't already, I'd also suggest checking out logrus. It seems to be the de facto standard for logging in Go, and supports things like structured data. It's pluggable, you can use any logging backend you want with it. GitLab uses logrus, wrapped in their own wrapper.
I would avoid using logrus since there are many different go loggers out there and each has its interface, plus it would bloat the package. Perhaps we could make this Logger interface structured instead |
Yes, I think that would be a good idea too, and then on GitLab's side we could simply add a Logrus backend for your |
Cc: @ashmckenzie |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, one minor suggestion.
Pass a custom logger to log messages
We're currently processing your upload. This comment will be updated when the results are available. |
This reverts commit bb5f31e.
Pass a custom logger to log messages