cosmos / ibc-go

Inter-Blockchain Communication Protocol (IBC) implementation in Golang.
https://ibc.cosmos.network/
MIT License
553 stars 593 forks source link

Replace clang-format with buf format in proto-format #3651

Open DimitrisJim opened 1 year ago

DimitrisJim commented 1 year ago

Summary

For formatting proto files, we currently use clang-format. Afaik, this was inherited from the sdk with #2672. Alternatively, we could consider using buf which is already used in the Makefile for the proto-lint and proto-check-breaking rules.

See https://github.com/cosmos/ibc-go/pull/3640#discussion_r1204199620 for some context on why this issue was opened.

Pros

Cons

Here's a sample diff from formatting client.proto with buf:

 package ibc.core.client.v1;

-option go_package = "github.com/cosmos/ibc-go/v7/modules/core/02-client/types";
-
-import "gogoproto/gogo.proto";
-import "google/protobuf/any.proto";
 import "cosmos/upgrade/v1beta1/upgrade.proto";
 import "cosmos_proto/cosmos.proto";
+import "gogoproto/gogo.proto";
+import "google/protobuf/any.proto";
+
+option go_package = "github.com/cosmos/ibc-go/v7/modules/core/02-client/types";

 // IdentifiedClientState defines a client state with an additional client
 // identifier field.
@@ -41,7 +41,7 @@ message ClientConsensusStates {
 // handler may fail if the subject and the substitute do not match in client and
 // chain parameters (with exception to latest height, frozen height, and chain-id).
 message ClientUpdateProposal {
-  option (gogoproto.goproto_getters)         = false;
+  option (gogoproto.goproto_getters) = false;
   option (cosmos_proto.implements_interface) = "cosmos.gov.v1beta1.Content";
   // the title of the update proposal
   string title = 1;
@@ -57,14 +57,14 @@ message ClientUpdateProposal {
 // UpgradeProposal is a gov Content type for initiating an IBC breaking
 // upgrade.
 message UpgradeProposal {
-  option (gogoproto.goproto_getters)         = false;
-  option (gogoproto.goproto_stringer)        = false;
-  option (gogoproto.equal)                   = true;
+  option (gogoproto.goproto_getters) = false;
+  option (gogoproto.goproto_stringer) = false;
+  option (gogoproto.equal) = true;
   option (cosmos_proto.implements_interface) = "cosmos.gov.v1beta1.Content";

-  string                      title       = 1;
-  string                      description = 2;
-  cosmos.upgrade.v1beta1.Plan plan        = 3 [(gogoproto.nullable) = false];
+  string title = 1;
+  string description = 2;
+  cosmos.upgrade.v1beta1.Plan plan = 3 [(gogoproto.nullable) = false];

   // An UpgradedClientState must be provided to perform an IBC breaking upgrade.
   // This will make the chain commit to the correct upgraded (self) client state
@@ -86,7 +86,7 @@ message UpgradeProposal {
 // height continues to be monitonically increasing even as the RevisionHeight
 // gets reset
 message Height {
-  option (gogoproto.goproto_getters)  = false;
+  option (gogoproto.goproto_getters) = false;
   option (gogoproto.goproto_stringer) = false;

   // the revision that the client is currently on

For Admin Use

damiannolan commented 1 year ago

Hmm, I must admit I do kind of like the formatting clang-format provides wrt to spacing and aligning field number ordering. But I'd be open to looking into this a bit more 👍