From 623df2b3cfa7be8deac827483af012babb820ac1 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 10 Jun 2024 05:14:47 -0700 Subject: [PATCH] added more stuff --- Makefile | 2 + README.md | 435 +++++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 321 insertions(+), 116 deletions(-) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..259dadf --- /dev/null +++ b/Makefile @@ -0,0 +1,2 @@ +fmt: + prettier -w README.md diff --git a/README.md b/README.md index 5c0cc43..d1fc409 100644 --- a/README.md +++ b/README.md @@ -1,33 +1,196 @@ -# My Code Styleguide +# sneak/styleguide + +The following is the first released version of my personal code styleguide. + +Only the Go portion is "complete". The others are mostly just +placeholders. + +Feedback and suggestions are not only welcome but explicitly encouraged. + +[sneak@sneak.berlin](mailto:sneak@sneak.berlin) + +# My 2024 Code Styleguide + +## All + +1. Every project/repo should have a `Makefile` in the root. At a minimum, + `make clean`, `make run`, `make fmt`, and `make test` should work. Choose + a sane default target (`test` for libraries, `run` or `publish` for + binaries). `fmt` should invoke the appropriate formatters for the files + in the repo, such as `go fmt`, `prettier`, `black`, etc. Other standard + `Makefile` targets include `deploy`, `lint`. Consider the `Makefile` the + official documentation about how to operate the repository. + +1. If it's possible to write a `Dockerfile`, include at least a simple one. + It should be possible to build and run the project with `docker build .`. + +1. For F/OSS-licensed software, try to include the full source code of the + current version (and any dependencies, such as vendored dependencies) in + the docker image. They're small and should be included with the binary. + +1. Under no circumstances should any credentials or secrets ever be + committed to any repository, even private ones. Store secrets in + environment variables, and if they are absolutely required, check on + startup to make sure they are set/non-default and complain loudly if not. + Exception, sometimes: public keys. (Public keys can still sometimes be + secrets for operational security reasons.) + +1. Avoid nesting `if` statements. If you have more than one level of + nesting, consider inverting the condition and using `return` to exit + early. + +1. Almost all services/servers should accept their configuration via + environment variables. Only go full config file if absolutely necessary. + +1. For services/servers, log JSON to stdout. This makes it easier to parse + and aggregate logs when run under `docker`. Use structured logging + whenever possible. You may detect if the output is a terminal and + pretty-print the logs in that case. + +1. Debug mode is enabled by setting the environment variable `DEBUG` to a + non-empty string. This should enable verbose logging and such. It will + never be enabled in prod. + +1. For services/servers, make a healthcheck available at + `/.well-known/healthcheck`. This is out of spec but it is my personal + standard. This should return a 200 OK if the service is healthy, along + with a JSON object containing the service's name, uptime, and any other + relevant information, and a key of "status" with a value of "ok" if the + service is healthy. Make sure that in the event of a failure, the service + returns a 5xx status code for that route. + +1. If possible, for services/servers, include a /metrics endpoint that + returns Prometheus-formatted metrics. This is not required for all + services, but is a nice-to-have. + +## Bash / Shell + +1. Use `[[` instead of `[` for conditionals. It's a shell builtin and + doesn't have to execute a separate process. + +1. Use `$( )` instead of backticks. It's easier to read and nest. + +1. Use `#!/usr/bin/env bash` as the shebang line. This allows the script to + be run on systems where `bash` is not in `/bin`. + +1. Use `set -euo pipefail` at the top of every script. This will cause the + script to exit if any command fails, and will cause the script to exit if + any variable is used before it is set. + +1. Use `pv` for progress bars when piping data through a command. This makes + it easier to see how much data has been processed. + +1. Put all code in functions, even a main function. Define all functions + then call main at the bottom of the file. + +## JavaScript / ECMAScript / ES6 + +1. Use `const` for everything. If you need to reassign, use `let`. Never + use `var`. + +1. Use yarn for package management, avoid using npm. + +1. Use LTS node versions. + +1. Use `prettier` for code formatting, with four spaces for indentation. + +1. At a minimum, `npm run test` and `npm run build` should work (complete + the appropriate scripts in `package.json`). The `Makefile` should call + these, do not duplicate the scripts in the `Makefile`. + +## Docker Containers (for services) + +1. Use `runit` with `runsvinit` as the entrypoint for all containers. This + allows for easy service management and logging. In startup scripts + (`/etc/service/*/run`) in the container, put a `sleep 1` at the top of + the script to avoid spiking the cpu in the case of a fast-exiting process + (such as in an error condition). This also limits the maximum number of + error messages in logs to 86400/day. ## Python -1. Format all code with `black`. +1. Format all code with `black`, with four space indents. 2. Put all code in functions. If you are writing a script, put the script in a function called `main` and call `main()` at the end of the script using the standard invocation: - ```python - if __name__ == "__main__": - main() - ``` + ```python + if __name__ == "__main__": + main() + ``` ## Golang -1. Any project that has more than 2 or 3 modules should use the `uber/fx` DI - framework to keep things tidy. +1. Try to hard wrap long lines at 77 characters or less. -2. Don't commit anything that hasn't been `go fmt`'d. The only exception is +1. Don't commit anything that hasn't been `go fmt`'d. The only exception is when committing things that aren't yet syntactically valid, which should - only happen pre-v0.0.1 or on a non-main branch. + only happen pre-v0.0.1 or on a non-`main` branch. -3. Even if you are planning to deal with only positive integers, use +1. Even if you are planning to deal with only positive integers, use `int`/`int64` types instead of `uint`/`uint64` types. This is for consistency and compatibility with the standard library; it's better than casting all the time. -4. Try to use zerolog for logging. It's fast and has a nice API. For +1. Any project that has more than 2 or 3 modules should use the + `go.uber.org/fx` dependency injection framework to keep things tidy. + +1. If you have to choose between readable and clever, opt for readable. + It's ok to make the code less concise or slightly less idiomatic if you + can keep it dead simple. + +1. Embed the git commit hash into the binary and include it in startup logs + and in health check output. This is to make it easier to correlate + running instances with their code. Do not include build time or build + user, as these will make the build nondeterministic. + + Example relevant Makefile sections: + + Given a `main.go` like: + + ```go + package main + + import ( + "fmt" + ) + + var ( + Version string + Buildarch string + ) + + func main() { + fmt.Printf("Version: %s\n", Version) + fmt.Printf("Buildarch: %s\n", Buildarch) + } + ``` + + ```make + VERSION := $(shell git describe --always --dirty) + BUILDARCH := $(shell uname -m) + + GOLDFLAGS += -X main.Version=$(VERSION) + GOLDFLAGS += -X main.Buildarch=$(BUILDARCH) + + # osx can't statically link apparently?! + ifeq ($(UNAME_S),Darwin) + GOFLAGS := -ldflags "$(GOLDFLAGS)" + endif + + ifneq ($(UNAME_S),Darwin) + GOFLAGS = -ldflags "-linkmode external -extldflags -static $(GOLDFLAGS)" + endif + + ./httpd: ./pkg/*/*.go ./internal/*/*.go cmd/httpd/*.go + go build -o $@ $(GOFLAGS) ./cmd/httpd/*.go + ``` + +1. Avoid obvious footguns. For example, use range instead of for loops for + iterating. + +1. Try to use zerolog for logging. It's fast and has a nice API. For smaller/quick projects, the standard library's `log` package (and specifically `log/slog`) is fine. In that case, log structured logs whenever possible, and import `sneak.berlin/go/simplelog` to configure @@ -46,79 +209,118 @@ } ``` -5. Write at least a single test to check compilation. The test file can be - empty, but it should exist. This is to ensure that `go test ./...` will - always function as a syntax check at a minimum. +1. Commit at least a single test file to check compilation. The test file + can be empty, but it should exist. This is to ensure that `go test +./...` will always function as a syntax check at a minimum. -6. For anything beyond a simple script or tool, or anything that is going to +1. Full TDD and coverage isn't that important, but when fixing a specific + bug, try to write a test that reproduces the bug before fixing it. This + will help ensure that the bug doesn't come back later, and crystallizes + the experience of discovering the bug and the resulting fix into the + repository's history. + +1. For anything beyond a simple script or tool, or anything that is going to run in any sort of "production" anywhere, make sure it passes `golangci-lint`. -7. Write a `Dockerfile` for every repo, even if it only runs the tests and +1. Write a `Dockerfile` for every repo, even if it only runs the tests and linting. `docker build .` should always make sure that the code is in an able-to-be-compiled state, linted, and any tests run. The Docker build should fail if linting doesn't pass. -8. Include a `Makefile` with targets for at least `clean` and `test`. If +1. Include a `Makefile` with targets for at least `clean` and `test`. If there are multiple binaries, include a target for each binary. If there are multiple binaries, include a target for `all` that builds all binaries. -9. If you are writing a single-module library, `.go` files are okay in the +1. If you are writing a single-module library, `.go` files are okay in the repo root. -10. If you are writing a multi-module project, put all `.go` files in a +1. If you are writing a multi-module project, put all `.go` files in a `pkg/` or `internal/` directory. This is to keep the root clean and to make it easier to see what is a library and what is a binary. -11. Binaries go in `cmd/` directories. Each binary should have its own +1. Binaries go in `cmd/` directories. Each binary should have its own directory. This is to keep the root clean and to make it easier to see what is a library and what is a binary. Only package `main` files should be in `cmd/*` directories. -12. Keep the `main()` function as small as possible. +1. Keep the `main()` function as small as possible. -13. Keep the `main` package as small as possible. Move as much code as is - feasible to a library package. +1. Keep the `main` package as small as possible. Move as much code as is + feasible to a library package, even if it's an internal one. `main` is + just an entrypoint to your code, not a place for implementations. + Exception: single-file scripts. -14. HTTP HandleFuncs should be returned from methods or functions that need +1. HTTP HandleFuncs should be returned from methods or functions that need to handle HTTP requests. Don't use methods or our top level functions as handlers. -15. Provide a .gitignore file that ignores at least `*.log`, `*.out`, and +1. Provide a .gitignore file that ignores at least `*.log`, `*.out`, and `*.test` files, as well as any binaries. -16. Constructors should be called `New()` whenever possible. +1. Constructors should be called `New()` whenever possible. `modulename.New()` works great if you name the packages properly. -17. Don't make packages too big. Break them up. +1. Don't make packages too big. Break them up. -18. Don't make functions or methods too big. Break them up. +1. Don't make functions or methods too big. Break them up. -19. Use descriptive names for functions and methods. Don't be afraid to +1. Use descriptive names for functions and methods. Don't be afraid to make them a bit long. -20. Use descriptive names for modules and filenames. Avoid generic names +1. Use descriptive names for modules and filenames. Avoid generic names like `server`. `util` is banned. -21. Constructors should take a Params struct if they need more than 1-2 +1. Constructors should take a Params struct if they need more than 1-2 arguments. Positional arguments are an endless source of bugs and should be avoided whenever possible. -22. Use `context.Context` for all functions that need it. If you don't need +1. Use `context.Context` for all functions that need it. If you don't need it, you can pass `context.Background()`. Anything long-running should get and abide by a Context. A context does not count against your number of function or method arguments for purposes of calculating whether or not you need a Params struct, because the `ctx` is always first. -23. Contexts are always named `ctx`. +1. Contexts are always named `ctx`. -24. Use `context.WithTimeout` or `context.WithDeadline` for any function +1. Use `context.WithTimeout` or `context.WithDeadline` for any function that could potentially run for a long time. This is especially true for any function that makes a network call. Sane timeouts are essential. -25. Avoid global state, especially global variables. If you need to store +1. If a structure/type is only used in one function or method, define it + there. If it's used in more than one, define it in the package. Keep it + close to its usages. For example: + + ```go + func (m *Mothership) tvPost() http.HandlerFunc { + + type MSTVRequest struct { + URL string `json:"URL"` + } + + type MSTVResponse struct { + } + + return func(w http.ResponseWriter, r *http.Request) { + // parse json from request + var reqParsed MSTVRequest + err = json.NewDecoder(r.Body).Decode(&reqParsed) + ... + + if err != nil { + SendErrorResponse(w, MSGenericError) + return + } + + log.Info().Msgf("Casting to %s: %s", tvName, streamURL) + SendSuccessResponse(w, &MSTVResponse{}) + } + } + ``` + +1. Avoid global state, especially global variables. If you need to store state that is global to your launch or application instance, use a package `globals` or `appstate` with a struct and a constructor and require it as a dependency in your constructors. This will allow @@ -127,10 +329,10 @@ graph allows for it, put it in the main struct/object of your application, but remember that this harms testability. -26. Package-global "variables" are ok if they are constants, such as static +1. Package-global "variables" are ok if they are constants, such as static strings or integers or errors. -27. Whenever possible, avoid hardcoding numbers or values in your code. Use +1. Whenever possible, avoid hardcoding numbers or values in your code. Use descriptively-named constants instead. Recall the famous SICP quote: "Programs must be written for people to read, and only incidentally for machines to execute." Rather than comments, a descriptive constant name @@ -149,143 +351,144 @@ } ``` -28. Define your struct types near their constructors. +1. Define your struct types near their constructors. -29. Define your interface types near the functions that use them, or if you +1. Define your interface types near the functions that use them, or if you have multiple conformant types, put the interface(s) in their own file. -30. Define errors as package-level variables. Use a descriptive name for the +1. Define errors as package-level variables. Use a descriptive name for the error. Use `errors.New` to create the error. If you need to include additional information in the error, use a struct that implements the `error` interface. -31. Use lowerCamelCase for local function/variable names. Use UpperCamelCase +1. Use lowerCamelCase for local function/variable names. Use UpperCamelCase for type names, and exported function/variable names. Use snake_case for JSON keys. Use lowercase for filenames. -32. Explicitly specify UTC for datetimes unless you have a very good reason +1. Explicitly specify UTC for datetimes unless you have a very good reason not to. Use `time.Now().UTC()` to get the current time in UTC. -33. String dates should always be ISO8601 formatted. Use `time.Time.Format` +1. String dates should always be ISO8601 formatted. Use `time.Time.Format` with `time.RFC3339` to get the correct format. -34. Use `time.Time` for all date and time values. Do not use `int64` or +1. Use `time.Time` for all date and time values. Do not use `int64` or `string` for dates or times internally. -35. When using `time.Time` in a struct, use a pointer to `time.Time` so that +1. When using `time.Time` in a struct, use a pointer to `time.Time` so that you can differentiate between a zero value and a null value. -36. Use `time.Duration` for all time durations. Do not use `int64` or +1. Use `time.Duration` for all time durations. Do not use `int64` or `string` for durations internally. -37. When using `time.Duration` in a struct, use a pointer to `time.Duration` +1. When using `time.Duration` in a struct, use a pointer to `time.Duration` so that you can differentiate between a zero value and a null value. -38. Whenever possible, in argument types and return types, try to use +1. Whenever possible, in argument types and return types, try to use standard library interfaces instead of concrete types. For example, use `io.Reader` instead of `*os.File`. Tailor these to the needs of the specific function or method. Examples: - - **`io.Reader`** instead of `*os.File`: + - **`io.Reader`** instead of `*os.File`: - - `io.Reader` is a common interface for reading data, which can be - implemented by many types, including `*os.File`, `bytes.Buffer`, - `strings.Reader`, and network connections like `net.Conn`. + - `io.Reader` is a common interface for reading data, which can be + implemented by many types, including `*os.File`, `bytes.Buffer`, + `strings.Reader`, and network connections like `net.Conn`. - - **`io.Writer`** instead of `*os.File` or `*bytes.Buffer`: + - **`io.Writer`** instead of `*os.File` or `*bytes.Buffer`: - - `io.Writer` is used for writing data. It can be implemented by - `*os.File`, `bytes.Buffer`, `net.Conn`, and more. + - `io.Writer` is used for writing data. It can be implemented by + `*os.File`, `bytes.Buffer`, `net.Conn`, and more. - - **`io.ReadWriter`** instead of `*os.File`: + - **`io.ReadWriter`** instead of `*os.File`: - - `io.ReadWriter` combines `io.Reader` and `io.Writer`. It is often - used for types that can both read and write, such as `*os.File` - and `net.Conn`. + - `io.ReadWriter` combines `io.Reader` and `io.Writer`. It is often + used for types that can both read and write, such as `*os.File` + and `net.Conn`. - - **`io.Closer`** instead of `*os.File` or `*net.Conn`: + - **`io.Closer`** instead of `*os.File` or `*net.Conn`: - - `io.Closer` is used for types that need to be closed, including - `*os.File`, `net.Conn`, and other resources that require cleanup. + - `io.Closer` is used for types that need to be closed, including + `*os.File`, `net.Conn`, and other resources that require cleanup. - - **`io.ReadCloser`** instead of `*os.File` or `http.Response.Body`: + - **`io.ReadCloser`** instead of `*os.File` or `http.Response.Body`: - - `io.ReadCloser` combines `io.Reader` and `io.Closer`, and is - commonly used for types like `*os.File` and `http.Response.Body`. + - `io.ReadCloser` combines `io.Reader` and `io.Closer`, and is + commonly used for types like `*os.File` and `http.Response.Body`. - - **`io.WriteCloser`** instead of `*os.File` or `*gzip.Writer`: + - **`io.WriteCloser`** instead of `*os.File` or `*gzip.Writer`: - - `io.WriteCloser` combines `io.Writer` and `io.Closer`. It is used - for types like `*os.File` and `gzip.Writer`. + - `io.WriteCloser` combines `io.Writer` and `io.Closer`. It is used + for types like `*os.File` and `gzip.Writer`. - - **`io.ReadWriteCloser`** instead of `*os.File` or `*net.TCPConn`: + - **`io.ReadWriteCloser`** instead of `*os.File` or `*net.TCPConn`: - - `io.ReadWriteCloser` combines `io.Reader`, `io.Writer`, and - `io.Closer`. Examples include `*os.File` and `net.TCPConn`. + - `io.ReadWriteCloser` combines `io.Reader`, `io.Writer`, and + `io.Closer`. Examples include `*os.File` and `net.TCPConn`. - - **`fmt.Stringer`** instead of implementing a custom `String` method: + - **`fmt.Stringer`** instead of implementing a custom `String` method: - - `fmt.Stringer` is an interface for types that can convert - themselves to a string. Any type that implements the `String() -string` method satisfies this interface. + - `fmt.Stringer` is an interface for types that can convert + themselves to a string. Any type that implements the `String() - - **`error`** instead of custom error types: + string` method satisfies this interface. - - The `error` interface is used for representing errors. Instead of - defining custom error types, you can use the `errors.New` - function or the `fmt.Errorf` function to create errors. + - **`error`** instead of custom error types: - - **`net.Conn`** instead of `*net.TCPConn` or `*net.UDPConn`: + - The `error` interface is used for representing errors. Instead of + defining custom error types, you can use the `errors.New` + function or the `fmt.Errorf` function to create errors. - - `net.Conn` is a generic network connection interface that can be - implemented by TCP, UDP, and other types of network connections. + - **`net.Conn`** instead of `*net.TCPConn` or `*net.UDPConn`: - - **`http.Handler`** instead of custom HTTP handlers: + - `net.Conn` is a generic network connection interface that can be + implemented by TCP, UDP, and other types of network connections. - - `http.Handler` is an interface for handling HTTP requests. - Instead of creating custom handler types, you can use types that - implement the `ServeHTTP(http.ResponseWriter, *http.Request)` - method. + - **`http.Handler`** instead of custom HTTP handlers: - - **`http.HandlerFunc`** instead of creating a new type: + - `http.Handler` is an interface for handling HTTP requests. + Instead of creating custom handler types, you can use types that + implement the `ServeHTTP(http.ResponseWriter, *http.Request)` + method. - - `http.HandlerFunc` is a type that allows you to use functions as - HTTP handlers by implementing the `http.Handler` interface. + - **`http.HandlerFunc`** instead of creating a new type: - - **`encoding.BinaryMarshaler` and `encoding.BinaryUnmarshaler`** - instead of custom marshal/unmarshal methods: + - `http.HandlerFunc` is a type that allows you to use functions as + HTTP handlers by implementing the `http.Handler` interface. - - These interfaces are used for binary serialization and - deserialization. Implementing these interfaces allows types to - be encoded and decoded in a standard way. + - **`encoding.BinaryMarshaler` and `encoding.BinaryUnmarshaler`** + instead of custom marshal/unmarshal methods: - - **`encoding.TextMarshaler` and `encoding.TextUnmarshaler`** instead - of custom text marshal/unmarshal methods: + - These interfaces are used for binary serialization and + deserialization. Implementing these interfaces allows types to + be encoded and decoded in a standard way. - - These interfaces are used for text-based serialization and - deserialization. They are useful for types that need to be - represented as text. + - **`encoding.TextMarshaler` and `encoding.TextUnmarshaler`** instead + of custom text marshal/unmarshal methods: - - **`sort.Interface`** instead of custom sorting logic: + - These interfaces are used for text-based serialization and + deserialization. They are useful for types that need to be + represented as text. - - `sort.Interface` is an interface for sorting collections. By - implementing the `Len`, `Less`, and `Swap` methods, you can sort - any collection using the `sort.Sort` function. + - **`sort.Interface`** instead of custom sorting logic: - - **`flag.Value`** instead of custom flag parsing: - - `flag.Value` is an interface for defining custom command-line - flags. Implementing the `String` and `Set` methods allows you to - use custom types with the `flag` package. + - `sort.Interface` is an interface for sorting collections. By + implementing the `Len`, `Less`, and `Swap` methods, you can sort + any collection using the `sort.Sort` function. -39. Avoid using `panic` in library code. Instead, return errors to allow + - **`flag.Value`** instead of custom flag parsing: + - `flag.Value` is an interface for defining custom command-line + flags. Implementing the `String` and `Set` methods allows you to + use custom types with the `flag` package. + +1. Avoid using `panic` in library code. Instead, return errors to allow the caller to handle them. Reserve `panic` for truly exceptional conditions. -40. Use `defer` to ensure resources are properly cleaned up, such as +1. Use `defer` to ensure resources are properly cleaned up, such as closing files or network connections. Place `defer` statements immediately after resource acquisition. -41. When calling a function with `go`, wrap the function call in an +1. When calling a function with `go`, wrap the function call in an anonymous function to ensure it runs in the new goroutine context: Right: @@ -302,7 +505,7 @@ string` method satisfies this interface. go someFunction(arg1, arg2) ``` -42. Use `iota` to define enumerations in a type-safe way. This ensures that +1. Use `iota` to define enumerations in a type-safe way. This ensures that the constants are properly grouped and reduces the risk of errors. Example: @@ -343,7 +546,7 @@ string` method satisfies this interface. ) ``` -43. Don't hardcode big lists of things in your normal code. Either isolate +1. Don't hardcode big lists of things in your normal code. Either isolate lists in their own module/package and write some getters, or use a third party library. For example, if you need a list of country codes, you can use @@ -355,7 +558,7 @@ string` method satisfies this interface. probably too much. Compress the file before embedding and uncompress during the reading/parsing step for efficiency. -44. When storing numeric values that represent a number of units, either +1. When storing numeric values that represent a number of units, either include the unit in the variable name (e.g. `uptimeSeconds`, `delayMsec`, `coreTemperatureCelsius`), or use a type alias (that includes the unit name), or use a 3p library such as @@ -364,7 +567,7 @@ string` method satisfies this interface. [github.com/bcicen/go-units](https://github.com/bcicen/go-units) for temperatures (and others). The type system is your friend, use it. -45. Once you have a working program, run `go mod tidy` to clean up your +1. Once you have a working program, run `go mod tidy` to clean up your `go.mod` and `go.sum` files. Tag a v0.0.1 or v1.0.0. Push your `main` branch and tag(s). Subsequent work should happen on branches so that `main` is "always releasable". "Releasable" in this context means that