Improved main README and fixed npm install issues #14
Conversation
jonathan-casarrubias
force-pushed
the
jcasarrubias/built_in_grpc_types
branch
from
6315b54 to
49ac145
Compare
README.md
Outdated
| ## Overview | ||
| TODO. | ||
| The `@loopback/grpc` component enables LoopBack 4 as a Grpc Server. Also it provides with a @grpc decorator to define your RPC Method implementations from your Application Controllers. |
There was a problem hiding this comment.
Please add a url for gRPC. Change @grpc to @grpc.
| ```js | ||
| import {grpc} from '@loopback/grpc'; | ||
| // You create the following types according your own proto.file | ||
| import {Greeter, HelloRequest, HelloReply} from './types'; |
There was a problem hiding this comment.
Are these generated gRPC interfaces/classes?
There was a problem hiding this comment.
Look this awesome tool that @VMois found https://geotho.github.io/protobuf-to-typescript/
There was a problem hiding this comment.
It does convert great the messages but it looks service and rpc methods are not so good, but yeah the idea is to autogenerate those from the proto file
package greeterpackage;
syntax = "proto3";
service Greeter {
// Sends a greeting
rpc SayHello (HelloRequest) returns (HelloReply) {}
}
// The request message containing the user's name.
message HelloRequest {
string name = 1;
}
// The response message containing the greetings
message HelloReply {
string message = 1;
}
| export class MyGreeter implements Greeter { | ||
| // Tell LoopBack that this is a Service RPC implementation | ||
| @grpc() | ||
| SayHello(request: HelloRequest): HelloReply { |
There was a problem hiding this comment.
The method name is typically camelCase.
There was a problem hiding this comment.
Do we want to support separate parameters, such as sayHello(prefix: string, name: string)? The HelloRequest will have prefix and name properties. Maybe ES6 parameter destructuring is good enough.
There was a problem hiding this comment.
Yeah you are right I took it from the grpc examples but these are language agnostic, in this case the convention is camelCase so I'll change that. 👍
About the parameters I would rather not to do it as default. You know, I like the idea of passing an entire dependency instead of lots of parameters, also is the way you actually define RPC methods so is more consistent.
Do you see any benefit on passing each of the parameters instead of the entire object?
It could be a configurable option though, I would like the entire object to be default, but an option to pass the args as params should be easy to implement anyway.
|
Some nitpicks. LGTM. |
jonathan-casarrubias
force-pushed
the
jcasarrubias/built_in_grpc_types
branch
from
49ac145 to
a032fab
Compare
Improved main README and fixed npm install issues