jcelliott / turnpike

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

Begin work on adding RPC support: #52

Closed josephschorr closed 10 years ago

josephschorr commented 10 years ago
jcelliott commented 10 years ago

Thanks! Are you planning on adding any more commits to this PR, or should I merge it in?

josephschorr commented 10 years ago

I was planning on doing it in stages. If you think this PR is okay, please merge and then I'll start on the next one. My next PR should cover the REGISTER verb, with the following having UNREGISTER.

Also: Should I be writing tests for these changes and, if so, is there a preferred way to do so? I was unsure so I didn't in this PR.

jcelliott commented 10 years ago

Also: Should I be writing tests for these changes and, if so, is there a preferred way to do so? I was unsure so I didn't in this PR.

Glad you asked. A lot of the initial work on v2 was done without thorough testing, just so we could get something working quickly. But I would like to be more consistent with testing at this point. I've started using GoConvey for testing and it's actually quite pleasant to use. See my recent commit adding authentication support for an example (specifically in realm_test.go). GoConvey works seamlessly with standard Go tests though, so using it isn't a requirement.

josephschorr commented 10 years ago

Sounds good. I'll try to emulate your testing style going forward.

josephschorr commented 10 years ago

Anything I should add/change before this is merged?