libsv / go-spvchannels Goto Github PK

https://github.com/libsv/go-spvchannels/blob/master/Client.go#L62

Renaming the variable to 'out' should hopefully make it more obvious this is a response mapping.

Can you change the Client setups to use Functional Options instead of a config stuct. They are nicer to user and suers only need to set only what they need. I find option structs are confusing as you don't know if you need to set everything if you ony want to change 1 thing

https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

Another example can be found https://github.com/tonicpow/go-paymail/blob/master/client.go#L19

A convention we're trying to use for these functions is With... so here we'd have WithInsecure, WithBaseURL etc. Setting any of these would update the default config.

Please add comments to all public methods.

At the moment there is a maxMessages variable to trigger when the socket run ends. I don't think this is overly user friendly, how would I know what to set this to and how would I gracefully shut down the socket if I need to exit early.

Instead we should add a Close method to the socket client. This can be called to shutdown and end the socket.

I'm not 100% familiar with the gorilla websocket, but the close method would need to shutdown the gorilla socket and end the run loop and clear our any processing messages. You may need to employ a notify channel to exit the Run loop, I have an example of this here https://github.com/libsv/bitcoin-hc/blob/master/transports/zmq/headers.go#L60

Doing this means the caller can control when the Run loop ends and removes the need to a user to know in advance how many messages they want to process.

Small spelling mistake here https://github.com/libsv/go-spvchannels/blob/master/message.go#L29

There's a file at the root called svchannels.go, should this be spvchannels.go?

The linter will probably catch this, but adding in case it doesn't, rename all Acronyms in structs to be Uppercased, such as ID, JSON etc.

Remove the .vscode folder and add to gitignore.

There are some endpoints that have empty reply, we actually use an empty struct to define them. It is recommended to not use them and does not return anything if the reply is empty by definition

Add the same golangci config as found in go-bt - https://github.com/libsv/go-bt/blob/master/.golangci.yml

Add goreleaser config to the root directory, you can use the same config as found here https://github.com/libsv/go-bt/blob/master/.goreleaser.yml

Will also need to add this to the build pipeline, see https://github.com/libsv/go-bt/blob/master/.github/workflows/run-tests.yml#L46

Add the same licence as go-bt https://github.com/libsv/go-bt/blob/master/LICENSE Just update the dates :)

Can you change the Client setups to use Functional Options instead of a config stuct. They are nicer to user and suers only need to set only what they need. I find option structs are confusing as you don't know if you need to set everything if you ony want to change 1 thing

https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

Another example can be found https://github.com/tonicpow/go-paymail/blob/master/client.go#L19

A convention we're trying to use for these functions is With... so here we'd have WithInsecure, WithBaseURL etc. Setting any of these would update the default config.

We have public comments on all methods but these could be improved with more detailed explanations and code examples, I find a handy tool to use when writing these is https://bradleyjkemp.dev/godoc-playground/ which lets you write and interact with go docs in real time.

A handy guide for writing godocs is https://elliotchance.medium.com/godoc-tips-tricks-cda6571549b and also https://go.dev/blog/godoc.

When this repo is published the docs will appear here https://pkg.go.dev/github.com/libsv/go-spvchannels so we want it to be as friendly to users as possible. Yep, we also need to update our other repos but lets make this one the example to follow.