search

jcelliott / turnpike

Go implementation of a WAMP (Web Application Messaging Protocol) client and router
MIT License
258 stars 88 forks source link

Break package into client and router #93

Open jcelliott opened 9 years ago

jcelliott commented 9 years ago

It seems unfortunate that in order to use Turnpike's client API you have to import the entire router implementation. It seems like splitting the package would make things a bit cleaner. I propose to change the package structure to the following:

turnpike/
    client.go
    internal/
        message.go
        ...
    common/
        # not sure, anything common to client and router code that also should be exported
    router/
        router.go
        ...

With this structure, client would still be imported as .../turnpike, and router code would be imported as .../turnpike/router. It would use the Go 1.4 internal packages feature to prevent exporting unnecessary code.

I would like to get turnpike v2 to a stable release, and I think this would help move in that direction. @beatgammit @marshauf @DaniloMoura1 @thephred +anyone else: any suggestions or objections?

jcelliott commented 9 years ago

Related to this, I would like to un-export anything that doesn't need to be exported. I don't think the WAMP predefined error URIs (util.go) should be exported, as well as all the WAMP message types (message.go). Unless anyone needs those to be exported, I'm going to move them. It really clutters the package API (see the godoc result for turnpike).

marshauf commented 9 years ago

The net/http package has server and client related code in it for example. I used WAMP_ERROR_INVALID_ARGUMENT URI but no longer. The message types can be used when implementing a Authorizer.

jcelliott commented 9 years ago

The net/http package has server and client related code in it for example.

Good point.

I used WAMP_ERROR_INVALID_ARGUMENT URI but no longer.

If the errors should remain exported, we should probably make the names more idiomatic. Like ErrInvalidArgument, etc.

The message types can be used when implementing a Authorizer.

What if we put the message and error types in a wamp subpackage? Then you would use, e.g., wamp.ErrInvalidArgument and wamp.Invocation if you need them.

marshauf commented 9 years ago

Yes, all characters upper case is not idiomatic. Should be changed like you wrote.

The net/http package has all it's error values and status codes in the same package. I don't think the turnpike package is cluttered.

To improve and make the package more idiomatic, I suggest the use of: https://github.com/golang/lint It brings up many suggestions. In general we should run some tools like: "go test -race" It found 2 data races (on my local dev branch).