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

Fixing Comment Formatting According to Best Linter Practices #98

Merged
merged 3 commits into from
May 2, 2018
Merged

Fixing Comment Formatting According to Best Linter Practices #98

merged 3 commits into from
May 2, 2018

Conversation

rauljordan
Copy link
Contributor

Hi all,

This is a quick PR done to fix the comment formatting across our packages. I always get warnings from my linters in VSCode due to the problem of not commenting exported types. This is something we should do and is best practice. For example, if I create a type:

type Collation struct {
  header *CollationHeader
  body *CollationBody
}

We have to add a comment above that begins with the name of the type as follows:

// Collation defines the base struct for a sharding primitive
type Collation struct {
  header *CollationHeader
  body *CollationBody
}

Also removed punctuation from some comments in order to be consistent with our other comments throughout our packages.

@rauljordan rauljordan added the Enhancement New feature or request label May 2, 2018
@rauljordan rauljordan self-assigned this May 2, 2018
@rauljordan rauljordan added this to To do in Documentation and Tooling via automation May 2, 2018
@terencechain
Copy link
Member

LGTM. Do we need to do this for tests as well?

@rauljordan
Copy link
Contributor Author

No need to do this for test files @terenc3t

@terencechain
Copy link
Member

Great. Thanks for the fixes!

// * Verifies or deploys the sharding manager contract.
// Start the sharding client
// Connects to Geth node
// Verifies or deploys the sharding manager contract
func (c *shardingClient) Start() error {
Copy link
Member

Choose a reason for hiding this comment

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

Hey below can you add the comment to SMCTransactor() also

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

Choose a reason for hiding this comment

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

LGTM, I think we can merge it in

@rauljordan rauljordan merged commit 47c6537 into prysmaticlabs:master May 2, 2018
Documentation and Tooling automation moved this from To do to Done May 2, 2018
@prestonvanloon
Copy link
Member

Isn’t it best practice to end comments with punctuation? Like a sentence?

@terencechain terencechain mentioned this pull request May 14, 2018
3 tasks
prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018
Fixing Comment Formatting According to Best Linter Practices
prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018
Fixing Comment Formatting According to Best Linter Practices

Former-commit-id: 07a948d
prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018
Fixing Comment Formatting According to Best Linter Practices

Former-commit-id: 9cea91282e4f4abb12452789170508c6948513c1 [formerly 07a948d]
Former-commit-id: a779adf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants