pokt-network / pocket

Official implementation of the Pocket Network Protocol v1
https://pokt.network
MIT License
61 stars 33 forks source link

[Code Structure] Follow protobuf enum best practices #258

Open Olshansk opened 1 year ago

Olshansk commented 1 year ago

Objective

Not following best practices for protobuf enum naming.

Origin Document

In #245, the following change was made:

Screen Shot 2022-09-29 at 12 06 46 PM

In order to follow best practices, we should do the following:

Screen Shot 2022-09-29 at 12 08 20 PM

Goals

Deliverable

Non-goals / Non-deliverables

General issue deliverables

Testing Methodology


Creator: @Olshansk Co-Owners: @okdas + ?

EchoSpr commented 1 year ago

Can I take this on?

Olshansk commented 1 year ago

@nickatnight23 Definitely! Feel free to reach out either here or in discord if you need any help or clarifications.

The (dev guide](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md) is probably the best place to start, but if something is unclear or could be made better, please don't hesitate to point it out.

EchoSpr commented 1 year ago

Can I get some clarification on installing the dependencies? I am having trouble with this section- protoc-gen-go, protoc-go-inject-tag and mockgen by running make install_cli_deps. Not sure how to make the build first?

Olshansk commented 1 year ago

Can you try running this command: https://github.com/pokt-network/pocket/blob/main/Makefile#L99

If it works, feel free to update the dev guide as well!

EchoSpr commented 1 year ago

I have a question about the code base where the protobuf enums are located. Would this be the best place to ask?

Olshansk commented 1 year ago

For sure. Are you just having trouble finding them? This command might be a good starting point:

find . -name "*.proto" | xargs grep -i "enum " | grep -v "vendor"

Here's the output I got:

./consensus/types/proto/hotstuff_types.proto:enum HotstuffStep {
./consensus/types/proto/hotstuff_types.proto:enum HotstuffMessageType {
./shared/indexer/proto/transaction_indexer.proto:  int32 result_code = 4; // INVESTIGATE(andrew): look into having a `utility.Code` enum for this
./shared/indexer/proto/transaction_indexer.proto:  string error = 5; // INVESTIGATE(andrew): look into having a `utility.Error` enum for this
./shared/indexer/proto/transaction_indexer.proto:  string message_type = 8; // TODO(andrew): Add an enum for the different message types
./shared/debug/proto/events.proto:enum PocketTopic {
./shared/debug/proto/debug_message.proto:enum DebugMessageAction {
./persistence/proto/persistence_genesis.proto:enum PoolNames {
./persistence/proto/persistence_genesis.proto:enum ActorType {
./p2p/types/proto/p2p_config.proto:enum ConnectionType {
./utility/types/proto/actor.proto:enum PoolNames {
./utility/types/proto/actor.proto:enum ActorType {
./utility/types/proto/actor.proto:enum StakeStatus {
EchoSpr commented 1 year ago

Yes, I was having trouble finding them. Thank you.

EchoSpr commented 1 year ago

I am having trouble installing mockgen. It appears that everything else is installed.

Olshansk commented 1 year ago

What is the issue? Can you share some logs, a screenshot or what you've already tried / googled?