search

Kotlin / kotlinx-rpc

Add asynchronous RPC services to your multiplatform applications.
https://kotlin.github.io/kotlinx-rpc/
Apache License 2.0
749 stars 17 forks source link

Naming convention #88

Open edrd-f opened 5 months ago

edrd-f commented 5 months ago

Hello. First of all, thanks for the awesome work on this!

I wanted to bring up a minor issue regarding the naming convention used for the library classes. The official Kotlin coding conventions guide specifies the following:

When using an acronym as part of a declaration name, capitalize it if it consists of two letters (IOStream); capitalize only the first letter if it is longer (XmlFormatter, HttpInputStream).

(https://kotlinlang.org/docs/coding-conventions.html#choose-good-names)

Overall, the library is using capitalized RPC everywhere. I think for the RPC declaration it's ok, but RPCClient, RPCServer, for example, should actually be RpcClient and RpcServer.

Again, this is minor, but it would be nice to follow the official conventions. I can open a PR with this change if you all agree.

Mr3zee commented 5 months ago

Hey! Thank you for this issue and kind words!

We should indeed take a better look at how we follow conventions in our namings, as we, for example, have even larger caps namings like KRPCClient. 5 caps letters in a row! Scary!

Anyway, thanks for bringing that up. We will conduct internal design sessions where we will revisit all public APIs for this and other matters, so no need for a PR from your side. Can not promise, how long it could take, so hope it is not critical right now

edrd-f commented 5 months ago

No problem, this is absolutely not critical. Thank you for taking a look at this! 🙏

BenWoodworth commented 5 months ago

Here's a similar PR from kotlinx.serialization for reference: https://github.com/Kotlin/kotlinx.serialization/pull/262

Basically keeping it all caps when referencing the concept in docs, and changing the name in code (including the RPC interface) with type aliases/inline functions for some short-term source compatibility

And here's some (or all?) of the RPC declarations here for anyone curious