Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Naming convention #88

Closed
edrd-f opened this issue May 31, 2024 · 3 comments · Fixed by #236
Closed

Naming convention #88

edrd-f opened this issue May 31, 2024 · 3 comments · Fixed by #236
Milestone

Comments

@edrd-f
Copy link

edrd-f commented May 31, 2024

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
Copy link
Collaborator

Mr3zee commented Jun 3, 2024

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
Copy link
Author

edrd-f commented Jun 3, 2024

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

@BenWoodworth
Copy link

BenWoodworth commented Jun 15, 2024

Here's a similar PR from kotlinx.serialization for reference:
Kotlin/kotlinx.serialization#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

@Mr3zee Mr3zee added this to the 0.5.0 milestone Nov 5, 2024
@Mr3zee Mr3zee linked a pull request Nov 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants