You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by GitBox <gi...@apache.org> on 2022/10/09 02:48:48 UTC

[GitHub] [thrift] fishy commented on a diff in pull request #2700: THRIFT-5650: add uuid support in go

fishy commented on code in PR #2700:
URL: https://github.com/apache/thrift/pull/2700#discussion_r990723058


##########
lib/go/thrift/binary_protocol.go:
##########
@@ -26,6 +26,8 @@ import (
 	"fmt"
 	"io"
 	"math"
+
+	"github.com/google/uuid"

Review Comment:
   I don't think we want to use a third party library for this:
   
   1. so far we don't have any third party dependency (the only one is test only, not used by the library itself)
   2. there are several "competing" uuid libraries and we don't really want to force our user to a particular one.
   
    just `type UUID [16]byte` and it would be easy enough to be casted to the majority of the third party uuid libraries.
   
   the only thing we need to implement is converting from and to string.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org