[Nymphium]

This PR adds a complete service name (package.path.Service) to a service module.

Previously, the full service name had to be constructed using an arbitrary method from that service. This was inconvenient, as the name is a property of the service itself, not a specific method.

let module Method = Some.Service.Method in
let service_path =
   match Method.package_name with
   | None -> Method.service_name
   | Some p -> p ^ "." Method.sevice_name

With this PR, the name can be accessed directly from the service module, simplifying the logic.

let service_path = Some.Service.package_service_name

[andersfugmann]

Thanks for the PR.

I was considering if this logic is not better placed in Ocaml_protoc_plugin.Service. E.g:

let service_name (module Method : Rpc) =
  match Method.package_name with
  | None -> Method.service_name
  | Some p -> Printf.sprintf "%s.%s" p Method.service_name

But that can only be used with an actual service, i.e:

let service_name = Ocaml_protoc_plugin.service_name (module Echo.Echo.Call)
...

What is the usecase for this? Each Method already have a val name : string that contains the grpc path, e.g. /echo.Echo/Call. Is is important to understand the service path explicitly?

[Nymphium]

To register a service, currently we have to construct its full name by pulling information from one of its specific methods. This creates an unnecessary dependency—we want to register the service as a whole, so it feels strange to rely on a single method like echo.Echo.Call just to figure out the service's name. It also means we end up writing the same boilerplate logic to build that name every time.

For example, this is the current approach with https://github.com/dialohq/ocaml-grpc:

let service = Grpc_eio.Server.Service.(v () |> call  |> handle_request)
let name =
    match Echo.Echo.Call.package_name with
    | None -> Echo.Echo.Call.service_name
    | Some path -> path ^ "." ^ Echo.Echo.Call.service_name
 in
Grpc_eio.Server.add_service ~name ~service

This PR would allow us to do this instead, which is much more straightforward:

Grpc_eio.Server.add_service
  ~name:Echo.Echo.package_service_name
  ~service

This new approach might be better because it decouples the service's identity from its methods and makes the service module the single source of truth for its own name. The code becomes more readable, and we get to remove that repetitive logic.

[andersfugmann]

Thanks for the explanation! Much appreciated and thanks for the PR.
I wonder if it would make sense to move the service_name and package_name into the service itself aswell to avoid the redundency (and mark the old definitions as deprecated to retain backward compatibility). Any thoughs on this?

[Nymphium]

Thank you for approving it! I agree, that approach is much better 👍