-
Notifications
You must be signed in to change notification settings - Fork 206
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
docs: add style guide #1137
base: master
Are you sure you want to change the base?
docs: add style guide #1137
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
# Style Guide | ||
|
||
The eigenda repo consists majorly of Go and Solidity code, so we break it up into two sections: | ||
- [Go Style Guide](#go-style-guide) | ||
- [Solidity Style Guide](#solidity-style-guide) | ||
|
||
Our implicit rule is to defer to each language's respective style guides (see [resources](#resources) section). | ||
However, some disagreements keep coming up in PRs, so we document our conventions here to prevent too much bike-shedding. | ||
This style guide is thus by definition perpetually a work in progress. | ||
|
||
## Go Style Guide | ||
|
||
This document outlines the coding standards and best practices for our Go projects. | ||
|
||
### Code Organization | ||
|
||
Follow the [Ben Johnson](https://medium.com/@benbjohnson/structuring-applications-in-go-3b04be4ff091) repo structuring method: | ||
- Write interfaces in a file at the root of each package, and | ||
- Write every implementation of that interface in its own package in a subdirectory. | ||
|
||
#### Function Organization | ||
- Keep functions small and focused | ||
- Limit function length to ~50 lines where possible | ||
- Place the most important functions at the top of the file | ||
- Public functions that aren't methods should be placed in files with `_utils.go` suffix | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't always the case, i.e. constructors. Perhaps reword to say something like: "Public static functions that lack a tight coupling to a specific struct should be placed in files with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
### Error Handling | ||
|
||
- We follow Uber's [error-wrapping style](https://github.com/uber-go/guide/blob/master/style.md#error-wrapping). | ||
|
||
### Comments and Documentation | ||
|
||
- Document all exported functions, types, and variables. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree we should enforce docs for exported stuff, but I think we should also strongly encourage docs for ANYTHING non-trivial "Does the name of this thing tell me everything I could possibly need to know?" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also enforce param docs for exported functions |
||
- Use complete sentences with proper punctuation. | ||
- Keep comments concise and to the point. Avoid redundant comments that just restate the code. This means, in general, avoid LLM-generated comments. | ||
- Use your brain to bring in external context that only you have while writing the code. Code is often written once and read many times. Be mindful of the reader. | ||
|
||
Here is an example of what NOT to do: | ||
```go | ||
// service for users | ||
type UserService struct { | ||
db *sql.DB | ||
} | ||
|
||
// CreateUser creates a new user in the system. | ||
func (s *UserService) CreateUser(ctx context.Context, user *User) error { | ||
// ... implementation | ||
} | ||
``` | ||
Here is the same example with much more useful and complete comments. | ||
Note that its fine to have comments that are specific to the current implementation: | ||
```go | ||
// UserService handles all user-related operations. | ||
// | ||
// Safe for concurrent use by multiple goroutines. | ||
type UserService struct { | ||
// db is the database connection used by the service. | ||
// It is currently hardcoded with a connectino pool of size 10. | ||
// Note: we use SQL for flexibility for now, but may switch to NoSQL once our query patterns are fixed. | ||
db *sql.DB | ||
} | ||
|
||
// CreateUser creates a new user in the sql database. | ||
// The passed ctx should already have a deadline set, CreateUser does not add one. | ||
// It returns an error if the user already exists. | ||
func (s *UserService) CreateUser(ctx context.Context, user *User) error { | ||
// ... implementation | ||
} | ||
``` | ||
|
||
# Solidity Style Guide | ||
|
||
# Resources | ||
|
||
- [Effective Go](https://golang.org/doc/effective_go) | ||
- [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments) | ||
- [Go Style Guide](https://google.github.io/styleguide/go/) | ||
- [Go Proverbs](https://go-proverbs.github.io/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a section about small files, in addition to small functions
Mega files that contain multiple structs are no good. If a struct is non-trivial, it ought to have a dedicated file