search

aws / glide-for-redis

Apache License 2.0
152 stars 29 forks source link

Rename `Command` to `SingleCommand` in proto. #1385

Closed Yury-Fridlyand closed 1 month ago

Yury-Fridlyand commented 1 month ago

Issue #, if available: N/A

Description of changes: Without that rename we can't add command command to proto. Surprise!

Unfortunately, linters cause much more changes rather than simple rename.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Yury-Fridlyand commented 1 month ago

@barshaul, Should I add this to change log? It brings no changes for the user.

barshaul commented 1 month ago

Instead of this massive change, rename command to Cmd in protobuf. It's only used internally either way, and you can add a comment // commend.

Yury-Fridlyand commented 1 month ago

Amount of changes will be the same, unfortunately. Protobuf complains vs having the same name in a enum and in a class.

protobuf/redis_request.proto:178:5: "Command" is already defined in "redis_request".
protobuf/redis_request.proto:178:5: Note that enum values use C++ scoping rules, meaning that enum values are siblings of their type, not children of it.  Therefore, "Command" must be unique within "redis_request", not just within "RequestType".
barshaul commented 1 month ago

Amount of changes will be the same, unfortunately. Protobuf complains vs having the same name in a enum and in a class.

protobuf/redis_request.proto:178:5: "Command" is already defined in "redis_request".
protobuf/redis_request.proto:178:5: Note that enum values use C++ scoping rules, meaning that enum values are siblings of their type, not children of it.  Therefore, "Command" must be unique within "redis_request", not just within "RequestType".

as I said - rename the COMMAND command name to Cmd in protobuf, don't call it Command.


enum RequestType {
    ...
    CMD = 137; // COMMAND
}
Yury-Fridlyand commented 1 month ago

Got it, thanks.