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

proto: fix proto-linter comments for epoching/zoneconcierge #331

Merged
merged 5 commits into from
Mar 21, 2023

Conversation

SebastianElvis
Copy link
Member

Fixes BM-566

This PR fixes proto-linter in terms of the comments. It also updates the buf.yaml to enforce linting rules, executes make proto-format to format all proto files, and executes make proto-all to update all compiled stuff.

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Minor suggestions:

// CheckpointAddresses contains the addresses of the submitter and reporter of a
// given checkpoint submitter is the address of the submitter of the checkpoint
// to BTC reporter is the address of the submitter of the MsgInsertBTCSpvProof
// message to Babylon
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting for this comment seems off. e.g., submitter is the address of the submitter should have a period before it or be on a new line.

@@ -5,10 +5,13 @@ import "gogoproto/gogo.proto";

option go_package = "github.com/babylonchain/babylon/x/epoching/types";

// EventBeginEpoch is the event that an epoch has started
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// EventBeginEpoch is the event that an epoch has started
// EventBeginEpoch is an event emitted when an epoch has started

message EventBeginEpoch { uint64 epoch_number = 1; }

// EventEndEpoch is the event that an epoch has ended
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// EventEndEpoch is the event that an epoch has ended
// EventEndEpoch is an event emitted when an epoch has ended

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the same for the following comments, i.e. use is an event emitted instead of is the event. Think it better corresponds with the event nature here

@@ -23,26 +23,38 @@ service Msg {
returns (MsgWrappedBeginRedelegateResponse);
}

// MsgWrappedDelegate is the message for delegating some stakes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// MsgWrappedDelegate is the message for delegating some stakes
// MsgWrappedDelegate is the message for delegating stake

message MsgWrappedDelegateResponse {}

// MsgWrappedUndelegate is the message for undelegating some stakes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// MsgWrappedUndelegate is the message for undelegating some stakes
// MsgWrappedUndelegate is the message for undelegating stake

message MsgWrappedUndelegateResponse {}

// MsgWrappedDelegate is the message for moving some bonded stakes from a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// MsgWrappedDelegate is the message for moving some bonded stakes from a
// MsgWrappedDelegate is the message for moving bonded stake from a

proto/buf.yaml Outdated
@@ -13,6 +13,12 @@ lint:
- DEFAULT
- COMMENTS
- FILE_LOWER_SNAKE_CASE
# TODO Decide which comments we would like to enfore by linter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove the TODO?

@SebastianElvis SebastianElvis merged commit 82ac266 into dev Mar 21, 2023
@SebastianElvis SebastianElvis deleted the proto-lint-epoching-zc branch March 21, 2023 06:56
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 this pull request may close these issues.

2 participants