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(dependency): replace go-spew package #1499
Conversation
@kakkoyun PTAL |
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.
Thanks a lot for handling this.
I wonder how hard this is even without using a transitive dependency.
Could you maybe give it another shot with the standard library testing package?
Thanks for your review. Hmmm... Maybe not Diff string with the standard library testing package. Like:
|
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.
It's great that the test are green now. But we are losing some information.
I'd suggest adding the following file with proper licence headers to the testutils
package https://github.com/kylelemons/godebug/blob/master/diff/diff.go
https://pkg.go.dev/github.com/kylelemons/godebug@v1.1.0/diff
And updating the tests accordingly.
Or go back the previous version where you used go-cmp
.
In any case, good job. You're close :)
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
This reverts commit bfbe25e. Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
24026ac
to
a8e1df7
Compare
Thanks for your review. Please recheck :) |
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.
Great job 👏
Only one last thing.
I'd suggest adding the following file with proper licence headers to the testutils package https://github.com/kylelemons/godebug/blob/master/diff/diff.go
In my previous comment, I suggested copying over the diff.go
file from this repo rather than adding as a dependency. Remember our goal is to reduce the dependencies. Since it's a single file and simple enough. We can just copy the files (with licence headers) into the testutils
package.
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
Got it. Thanks. Fixed |
Signed-off-by: dongjiang1989 <dongjiang1989@126.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
use godebug package replace go-spew package
fix: #1490