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

[BUG] Proto files don't respect official style guide #86

Closed
kon14 opened this issue Mar 21, 2022 · 2 comments
Closed

[BUG] Proto files don't respect official style guide #86

kon14 opened this issue Mar 21, 2022 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@kon14
Copy link
Contributor

kon14 commented Mar 21, 2022

Describe the bug
While not necessarily breaking anything, Conduit should make sure its proto files abide by the official Protocol Buffers style guide.
I have yet to cross-check every single one of them, but at a first glance, it seems like we're mostly ignoring case conventions.
Namely snake_case for files (eg push-notifications.proto) and, sometimes, CamelCase for services and rpcs (eg Database's rpc findOne).

Expected behavior
Conduit's proto files should respect the official style guide, making sure they follow case conventions etc.

@kon14 kon14 added the bug Something isn't working label Mar 21, 2022
@sdimitris sdimitris self-assigned this Mar 23, 2022
@sdimitris
Copy link
Contributor

sdimitris commented Mar 24, 2022

Its not as simple as it seems. First of all changing the naming in the proto files will cause a huge collision
between Typescript and gRPC naming conventions.
Also , the gRPC's servers will crash because of rpc messages. ( Cause the naming will be changed).

gRPC Server Implementation

So, in order to solve this problem the gRPC server implementation should be changed.
ts-proto tool must be used in order to auto-generate the proper service/message interfaces.

Using this tool requires to create a Server Class for each module which implements the auto-generated interfaces

But there is a major problem in this solution:
It is difficult to implement a server class for the Router because the proto files are auto-generated.

@kkopanidis
Copy link
Contributor

Yeah the solution seems to require quite a bit of refactoring. I'm trying to understand if we tangled ourselves into this mess or it was bound to get messy either way. We'll need to have a design meeting on discord for this to figure out a proper approach to untangle this thing.

@kon14 kon14 mentioned this issue Apr 20, 2022
8 tasks
@kkopanidis kkopanidis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants