Add unused code back in to preserve compatibility with the sdk #522
Conversation
evan-forbes
requested review from
Bidon15 and
renaynay
and removed request for
liamsi
| tmsync "github.com/celestiaorg/celestia-core/libs/sync" | ||
| ) | ||
|
|
||
| // var maxNumberConnections = 2 |
There was a problem hiding this comment.
I don't quite get the idea why this comment is still left here stranded 😅
|
|
||
| // Read requests from conn and deal with them | ||
| func (s *SocketServer) handleRequests(closeConn chan error, conn io.Reader, responses chan<- *types.Response) { | ||
| var count int |
There was a problem hiding this comment.
It seems like a wisdom thought liner. Nevertheless, it would be nice if there is an explanation why this is only used for incrementing and nothing else 🤔
There was a problem hiding this comment.
wait, what's a wisdom thought liner? Also, I tend to agree, comment are usually very much appreciated.
There was a problem hiding this comment.
A wisdom thought liner is a 1 line of code that does something too magical that I can't get the understanding of it aka too wise for me 😸
|
|
||
| // Pull responses from 'responses' and write them to conn. | ||
| func (s *SocketServer) handleResponses(closeConn chan error, conn io.Writer, responses <-chan *types.Response) { | ||
| var count int |
| return connID | ||
| } | ||
|
|
||
| // deletes conn even if close errs |
There was a problem hiding this comment.
for comments, even on private funcs, I think it's nice to do:
// <func name> does X, Y, Z
|
@evan-forbes this is code copied and pasted from the cosmos sdk? |
|
This code was copied from the commit directly before it was deleted in celestia-core. It's needed for the sdk to compile. I tried to only copy the minimum code necessary, but will likely have to copy more as I broke a bunch of stuff in the sdk 😅 (see #28) so I'll leave this as a draft until I figure out how to fix all the breaking tests. Normally I like to incorporate as much feedback as possible, but I think in this very specific scenario where this code was copied from tendermint, I think it might be advantageous to leave the code alone so that we modify tendermint even less. If you think otherwise though, I'm very happy to incorporate the wonderful feedback you provided!! |
If we are applying the black box thinking to this, then for me it's for the best to just remeber/document somewhere what we actually think should be changed in the code-base. Or just make a PR with questions to tendermint itself 😅 |
Description
we need to restore some unused code to get the cosmos-sdk to compile. This should be reevaluated after we update to the latest versions of tendermint and the sdk.
Closes: #521