Protocol namespace #134
Protocol namespace #134
Conversation
src/scry/protocol/cancel_params.cr
Outdated
| JSON.mapping({ | ||
| id: Int32 | String, | ||
| }, true) | ||
| module Protocol |
There was a problem hiding this comment.
you can blend the 2 modules to module Scry::Protocol, and avoid adding another indentation inside
There was a problem hiding this comment.
I guess thats personal preference. I always prefer to keep module declarations separate, maybe its from my Ruby background where it was always advised to do it this way to avoid the situation where a module wasn't defined prior to writing the Path.
bmulvihill
force-pushed
the
protocol-namespace
branch
from
79e780d to
c8fdded
Compare
|
@bew on second thought I like that approach better. This isn't Ruby after all :) |
|
@bew any additional input regarding the namespace? |
| void_params = VoidParams.from_json("{}") | ||
| void_params.is_a?(VoidParams).should be_true | ||
| end | ||
| module Protocol |
There was a problem hiding this comment.
You can merge to Scry::Protocol here as well
|
Not sure why the build failed on osx: https://travis-ci.org/crystal-lang-tools/scry/jobs/417374390 |
|
@bew need to rebase this and pull in the 0.26.0 changes |
bmulvihill
force-pushed
the
protocol-namespace
branch
from
c8fdded to
580516c
Compare
This PR does two things to try and cleanup/organize the code some more.
Protocolnamespace, this should make things more clear about what is part of the LSP Protocolprotocol.crand requires that inscry.crinstead of requiring all of the protocol files throughout the program