OleksiienkoMykyta commented on PR #1850:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1850#issuecomment-2556857666
> I'm a bit confused about the `encVint` example, that's a private function
that should be easily moved to the internal package without much change right?
>
> Hmm moving every single type and function that is private to `internal` is
probably not the best idea because it will create a lot of cycle importing as
you mention, I realize this now. Maybe we should only try to move code to
`internal` that is a bit more isolated.
>
> The original GH issue mentions the marshaling code which should be easy to
move (marshaling related functions should be pretty much self contained). It
will still require some duplication (`Marshaler` interface probably?) but it
should be minimal.
>
> Moving the `eventDebouncer` should also be quite easy to do.
>
> Code in `frame.go` is a bit more tricky, there's a lot of the protocol
stuff that has to be part of the public API so moving it would result in a
decent amount of duplicated code. To remove this duplication I think it would
require some degree of refactoring so I think we can just leave it in the
public `gocql` module for now?
>
> Code in `token.go` seems like it can be moved pretty easily. Same with the
prepared cache. Same with code in `ring.go`.
>
> Conceptually code related to connections, control connection, connection
pools, etc. should also be able to be moved to an internal package but it might
require some refactoring to limit the amount of duplication... Control
connection and connection pool depends on the `Conn` type so any effort here
would start with the `Conn` type which also seems to be the most complicated
one to move because it actually has some public API stuff in there. It's
probably best to keep all of this connection related code in the `gocql` module
for now. **It might be easy to move the `writeCoalescer` type though.**
>
> There's parts of `metadata.go` that could maybe be moved to an internal
package but this one also seems a bit tricky, there's some public API
dependencies here.
>
> `logger.go` should be easily moved to `internal` but the interface
probably needs to be duplicated.
>
> It looks like `topology.go` code can also be moved easily? Not 100% here.
>
> In summary we should go on a case by case basis and if we see a lot of
public API dependencies on a particular part of the code then we should
probably leave it on the `gocql` package for now and focus on parts of the code
that don't have those dependencies.
`encVint` is an example of code that is easy to move, it was a little bit
unclear.
Ok, now the approach is much clearer, I think it will be easier to do this
way. The last question is should we keep this implementation in this brunch, or
just update this one? It could be useful for future implementation if we are
going to make such changes.
Thank you for the review, you helped me a lot!
Merry Xmas and have a great holidays!)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
