From a51c71004cc7a1962b63fa3f2c71ee087d54677c Mon Sep 17 00:00:00 2001 From: Julio Lopez <1953782+julio-lopez@users.noreply.github.com> Date: Wed, 29 Oct 2025 22:04:03 -0700 Subject: [PATCH] chore(ci): add copilot instructions (#4933) General and Go-specific instructions for Copilot --- .github/copilot-instructions.md | 408 ++++++++++++++++++ .../instructions/go.copilot-instructions.md | 352 +++++++++++++++ 2 files changed, 760 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/go.copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..9992005eb --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,408 @@ +# Kopia Copilot Instructions + +## When reviewing code, focus on: + +### Security Critical Issues +- Check for hardcoded secrets, API keys, or credentials +- Verify proper input validation and sanitization +- Review authentication and authorization logic + +### Code Quality Essentials +- Functions should be focused and appropriately sized +- Use clear, descriptive naming conventions +- Ensure proper error handling throughout + +### Performance Issues +- Spot inefficient loops and algorithmic issues +- Check for memory leaks and resource cleanup +- Review caching opportunities for expensive operations + +## Review Style +- Be specific and actionable in feedback +- Explain the rationale behind recommendations +- Acknowledge good patterns when you see them +- Ask clarifying questions when code intent is unclear + +Always prioritize security vulnerabilities and performance issues that could impact users. + +Always suggest changes to improve readability. + +## Repository Overview + +Kopia is a fast and secure open-source backup/restore tool written in **Go** that creates encrypted snapshots and saves them to remote storage. The repository is approximately 15MB with ~1,000 Go files. + +**Key Technologies:** +- **Backend:** Go (primary language) +- **Build System:** GNU Make with cross-platform support (Windows/Linux/macOS/ARM) +- **UI:** React-based HTML UI (embedded via go:embed, source at github.com/kopia/htmlui) +- **Desktop App:** Electron-based KopiaUI wrapper +- **Website:** Hugo static site generator + +## Build Commands + +### Setup (Required Before Building) +```bash +make -j4 ci-setup +``` +**Time:** ~30-60 seconds +**What it does:** Downloads Go modules, installs build tools (gotestsum, golangci-lint, hugo, node), and installs npm dependencies for the app. **ALWAYS run this after cloning or when build tools are missing.** + +## Linting + +**Run linter:** +```bash +make lint +``` +**Time:** ~3-4 minutes +**Linter:** golangci-lint with timeout of 1200s +**Config:** `.golangci.yml` (extensive configuration with 40+ enabled linters) + +**Auto-fix linting issues:** +```bash +make lint-fix +``` + +**Check code locks:** +```bash +make check-locks +``` +**Note:** Not available on linux/arm64 or linux/arm. + +**Check JavaScript/TypeScript formatting (in app directory):** +```bash +make check-prettier +``` + +**Important:** Linting is **NOT** run on linux/arm64 or linux/arm platforms to avoid issues. + +### Building Kopia CLI + +**Build without UI (faster for testing):** +```bash +make install-noui +``` +**Output:** `~/go/bin/kopia` +**Time:** ~5-10 seconds +**Use this for:** Testing CLI changes that don't involve the UI. + +**Race detector build:** +```bash +make install-race +``` +**Use this for:** Debugging race conditions. + +**Full build with embedded HTML UI:** +```bash +make install +``` +**Output:** `~/go/bin/kopia` +**Time:** ~10-20 seconds +**Note:** Embeds HTML UI from github.com/kopia/htmluibuild dependency. + +### Building KopiaUI Desktop App + +**Prerequisites:** Must build kopia CLI first (creates embedded binary). + +```bash +make kopia-ui +``` +**Output:** `dist/kopia-ui/` directory with platform-specific installers +**Time:** ~2-5 minutes +**Note:** Only works on amd64 architectures. On Linux, may require xvfb for headless testing. + +## Testing + +### Unit Tests (Standard) +```bash +make test +``` +**Time:** ~2-4 minutes +**Runs:** All unit tests with gotestsum, excludes TestIndexBlobManagerStress +**Timeout:** 1200s (20 minutes) per test +**Format:** pkgname-and-test-fails + +### Unit Tests with Coverage +```bash +make test-with-coverage +``` +**Output:** `coverage.txt` +**Time:** ~3-5 minutes +**Note:** Used by Code Coverage workflow. Sets KOPIA_COVERAGE_TEST=1 and GOEXPERIMENT=nocoverageredesign. + +### Index Blob Tests (Separate) +```bash +make test-index-blob-v0 +``` +**Runs:** TestIndexBlobManagerStress (excluded from standard tests due to duration) + +### Integration Tests +```bash +make build-integration-test-binary # Build test binary first +make integration-tests +``` +**Time:** ~5-10 minutes +**Requires:** KOPIA_INTEGRATION_EXE environment variable + +### CI Test Suites +```bash +make ci-tests # Runs: vet + test +make ci-integration-tests # Runs: robustness-tool-tests + socket-activation-tests +``` + +### Provider Tests (Cloud Storage) +```bash +make provider-tests PROVIDER_TEST_TARGET=... +``` +**Time:** 15 minutes timeout +**Requires:** KOPIA_PROVIDER_TEST=true, credentials for storage backend, rclone binary +**Note:** Tests various cloud storage providers (S3, Azure, GCS, etc.) + +### Other Test Types +- `make compat-tests` - Compatibility tests with older Kopia versions +- `make endurance-tests` - Long-running endurance tests (1 hour timeout) +- `make robustness-tests` - Robustness testing with FIO +- `make stress-test` - Stress tests (1 hour timeout) +- `make htmlui-e2e-test` - HTML UI end-to-end tests (10 minutes timeout) + +**Race Detector Tests:** +```bash +make test UNIT_TEST_RACE_FLAGS=-race UNIT_TESTS_TIMEOUT=1200s +``` + +## Common Issues & Workarounds + +### Build Issues + +1. **Missing build tools error:** Always run `make -j4 ci-setup` first after cloning. + +2. **Go version mismatch:** Kopia requires the Go toolchain with the version specified in go.mod. The `go-version-file` is used in GitHub Actions. + +3. **Platform-specific builds:** + - macOS: Creates universal binaries (AMD64 + ARM64) with `lipo` + - Windows: Requires chocolatey packages: make, zip, unzip, curl + - Linux ARM: Uses goreleaser for multi-arch builds on AMD64 host + +4. **KopiaUI build failures on ARM:** KopiaUI (Electron app) only builds on amd64. The build is skipped on ARM architectures. + +5. **Linting on ARM:** Linting and check-locks are skipped on linux/arm64 and linux/arm due to tool compatibility. + +### Test Issues + +1. **Test timeouts:** Default unit test timeout is 1200s (20 minutes). For race detector tests, explicitly set `UNIT_TESTS_TIMEOUT=1200s`. + +2. **Parallel execution:** Tests use `-parallel` flag (8 on amd64, 2 on ARM). Adjust with `PARALLEL` variable if needed. + +3. **Integration test binary:** Must build integration test binary explicitly with `make build-integration-test-binary` before running integration tests. + +4. **Provider tests require environment:** Provider tests need KOPIA_PROVIDER_TEST=true and rclone binary available. + +### Environment Variables + +**Important variables for CI/tests:** +- `UNIX_SHELL_ON_WINDOWS=true` - Required for Windows builds +- `KOPIA_COVERAGE_TEST=1` - Enable coverage testing +- `KOPIA_INTEGRATION_EXE` - Path to integration test binary +- `TESTING_ACTION_EXE` - Path to testing action binary +- `KOPIA_PROVIDER_TEST=true` - Enable provider tests +- `RCLONE_EXE` - Path to rclone binary for provider tests + +## Project Structure + +### Root Directory Files +- `main.go` - Entry point, uses kingpin for CLI parsing +- `Makefile` - Primary build system (GNU Make) +- `go.mod` / `go.sum` - Go module dependencies +- `.golangci.yml` - Linter configuration (extensive rules) +- `.gitignore` - Excludes dist/, .tools/, node_modules/, coverage files +- `BUILD.md` - Detailed build architecture documentation +- `README.md` - General project information + +### Source Directories + +**`cli/`** - CLI command implementations (~200 command files) +- Each command is in a separate file (e.g., `command_snapshot_create.go`) +- Uses kingpin v2 for command-line parsing +- Main entry via `app.go` + +**`repo/`** - Repository management and storage backends +- `repo/blob/` - Storage provider implementations (s3, azure, gcs, filesystem, etc.) +- `repo/content/` - Content-addressable storage layer +- `repo/format/` - Repository format and versioning +- `repo/manifest/` - Manifest management +- `repo/object/` - Object storage layer + +**`fs/`** - Filesystem abstraction layer +- `fs/localfs/` - Local filesystem implementation +- Supports snapshots, restore, and filesystem walking + +**`snapshot/`** - Snapshot creation and management +- `snapshot/snapshotmaintenance/` - Snapshot GC and maintenance +- `snapshot/upload/` - Upload logic and parallelization + +**`internal/`** - Internal packages (74 subdirectories) +- Utilities and shared code not for external use +- Examples: cache, crypto, compression, auth, server, etc. + +**`tests/`** - Integration and end-to-end tests +- `tests/end_to_end_test/` - E2E test scenarios +- `tests/robustness/` - Robustness testing framework +- `tests/tools/` - Test utilities and helpers + +**`tools/`** - Build and release tools +- `tools/gettool/` - Tool downloader (downloads versioned binaries) +- Various publishing scripts (apt, rpm, docker, homebrew) +- `tools/.tools/` - Downloaded build tools (gitignored) + +**`app/`** - Electron-based desktop application (KopiaUI) +- Node.js project with package.json +- Uses Electron Builder for packaging +- Resources in `app/resources/` and `app/public/` +- Embeds kopia server binary from `dist/kopia_*/kopia` + +**`site/`** - Hugo-based website (kopia.io) +- Build with `make -C site build` or `make website` +- Auto-generates CLI docs to `site/content/docs/Reference/Command-Line/` +- Development server: `make -C site server` (http://localhost:1313) + +### Configuration Files + +- `.golangci.yml` - Linter config with 40+ enabled linters, custom rules +- `.codecov.yml` - Code coverage reporting config +- `.goreleaser.yml` - Release automation config +- `.github/workflows/*.yml` - GitHub Actions workflows (19 workflow files) + +## GitHub Actions Workflows + +### Pull Request Checks (Always Run) + +1. **make.yml (Build)** - Builds on Windows/Linux/macOS/ARM + - Runs: `make ci-setup` → `make ci-build` + - Timeout: 40 minutes + - Creates artifacts: binaries, installers, packages + +2. **tests.yml** - Unit and integration tests on all platforms + - Runs: `make ci-setup` → `make test-index-blob-v0` → `make ci-tests` → `make ci-integration-tests` + - Uploads logs to artifacts + +3. **lint.yml** - Linting on ubuntu-latest and macos-latest + - Runs: `make lint` → `make check-locks` → `make check-prettier` + - Includes govulncheck for vulnerability scanning + +4. **code-coverage.yml** - Code coverage on ubuntu-latest + - Runs: `make test-with-coverage` + - Uploads to Codecov + +5. **race-detector.yml** - Race condition detection + - Runs: `make test UNIT_TEST_RACE_FLAGS=-race UNIT_TESTS_TIMEOUT=1200s` + +### Additional Workflows +- `providers-core.yml` / `providers-extra.yml` - Cloud storage provider tests +- `compat-test.yml` - Compatibility with older Kopia versions +- `stress-test.yml` - Stress testing +- `endurance-test.yml` - Long-running endurance tests +- `license-check.yml` - License compliance checking +- `dependency-review.yml` - Dependency security review +- `check-pr-title.yml` - PR title format validation + +### Workflow Tips +- **Build artifacts** are uploaded and can be downloaded from workflow runs +- **Logs** are uploaded to `.logs/**/*.log` on test failures +- **Concurrency:** All workflows use `cancel-in-progress: true` for the same ref +- **Scheduling:** Some workflows run weekly (Mondays at 8AM) + +## Development Workflow + +### Making Code Changes + +1. **Setup environment:** + ```bash + make -j4 ci-setup + ``` + +2. **Make your changes** to Go files + +3. **Build and test iteratively:** + ```bash + make install-noui # Fast build without UI + ~/go/bin/kopia --help # Test your changes + ``` + +4. **Lint your changes:** + ```bash + make lint + ``` + +5. **Run relevant tests:** + ```bash + make test # Unit tests + ``` + +6. **For HTML UI changes:** + - UI source is in separate repo: github.com/kopia/htmlui + - Pre-built UI imported from: github.com/kopia/htmluibuild + - To test local UI changes: `make install-with-local-htmlui-changes` (requires 3 repos checked out side-by-side) + +### Pre-Commit Checklist +- [ ] `make lint` passes (3-4 minutes) +- [ ] `make test` passes (2-4 minutes) +- [ ] Changes are formatted (gofumpt, gci enabled in linter) +- [ ] New packages: License check with `make license-check` + +### Code Style +- Uses golangci-lint with formatters: gci, gofumpt +- Imports organized: standard, default, localmodule +- No `time.Now()` outside clock/timetrack packages - use `clock.Now()` +- No `time.Since()` - use `timetrack.Timer.Elapsed()` +- No `filepath.IsAbs()` - use `ospath.IsAbs()` for Windows UNC support +- Tests use the `stretchr/testify` packages + +## Key Dependencies + +**Go modules (from go.mod):** +- Cloud providers: Azure SDK, AWS SDK (via minio), Google Cloud Storage +- CLI: alecthomas/kingpin/v2 +- Compression: klauspost/compress, klauspost/pgzip +- Testing: stretchr/testify, chromedp (for E2E) +- Observability: Prometheus client, OpenTelemetry +- UI: github.com/kopia/htmluibuild (pre-built React app) + +**Node.js dependencies (app/package.json):** +- electron-builder - Desktop app packaging +- electron-updater - Auto-updates +- React (via htmluibuild) - UI framework + +## Important Notes + +1. **Do not modify go.mod/go.sum manually** - Use `go get` to update dependencies. CI runs `git checkout go.mod go.sum` after ci-setup to revert local changes from tool downloads. + +2. **Build artifacts in dist/** - Gitignored. Contains platform-specific binaries and installers after `make ci-build` or `make goreleaser`. + +3. **Tools directory** - `tools/.tools/` is gitignored and populated by `make ci-setup`. Contains downloaded versions of gotestsum, linter, node, hugo, etc. + +4. **HTML UI is separate** - The HTML UI is maintained in github.com/kopia/htmlui and imported as a pre-built module. Don't try to find UI source in this repo. + +5. **Platform differences:** + - macOS: Creates universal binaries, requires Xcode command line tools + - Windows: Requires chocolatey tools, uses PowerShell for some commands + - Linux ARM: Builds via goreleaser on AMD64 host, creates ARM packages + +6. **Parallelism:** Makefiles use `-j4` for parallel execution. Tests use `-parallel 8` on amd64, `-parallel 2` on ARM. + +7. **Test binary paths:** + - Integration: `dist/testing_$(GOOS)_$(GOARCH)/kopia.exe` + - UI embedded: `dist/kopia_$(GOOS)_$(GOARCH)/kopia` (or universal binary on macOS) + +8. **Timeout configuration:** + - Linter: 1200s (20 minutes) + - Unit tests: 1200s (20 minutes) + - Integration tests: 300s (5 minutes) + - Provider tests: 15 minutes + - Stress/endurance: 3600s (1 hour) + +9. **Required tools installed by ci-setup:** + - gotestsum - Test runner with better output + - golangci-lint - Linter + - node - JavaScript runtime for app builds + - hugo - Static site generator for website + +10. **Trust these instructions** - These instructions have been validated by running all commands. Only search for additional information if something fails or if these instructions are incomplete or incorrect. diff --git a/.github/instructions/go.copilot-instructions.md b/.github/instructions/go.copilot-instructions.md new file mode 100644 index 000000000..c945717c9 --- /dev/null +++ b/.github/instructions/go.copilot-instructions.md @@ -0,0 +1,352 @@ +--- +applyTo: '**/*.go,**/go.mod,**/go.sum' +description: 'Instructions for writing Go code following idiomatic Go practices and community standards' + +--- + +# Go Development Instructions + + +Follow idiomatic Go practices and community standards when writing Go code. These instructions are based on [Effective Go](https://go.dev/doc/effective_go), [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments), and [Google's Go Style Guide](https://google.github.io/styleguide/go/guide). + +Refer to the linter configuration in `.golangci.yml` for style checks and standards + +## General Instructions + +- Write simple, clear, and idiomatic Go code +- Favor clarity and simplicity over cleverness +- Follow the principle of least surprise +- Keep the non-error path left-aligned (minimize indentation) +- Return early to reduce nesting +- Prefer early return over if-else chains; use `if condition { return }` pattern to avoid else blocks +- Make the zero value useful +- Write self-documenting code with clear, descriptive names +- Document exported types, functions, methods, and packages +- Use Go modules for dependency management +- Leverage and prefer the Go standard library when functionality exists instead of writing custom implementations (e.g., use `strings.Builder` for string concatenation, `filepath.Join` for path construction) +- Write comments in English +- Use allowed ASCII for identifiers; avoid using non-ASCII characters in identifiers +- Avoid using emoji in code and comments + +## Naming Conventions + +### Packages + +- Use lowercase, single-word package names +- Avoid underscores, hyphens, or mixedCaps +- Choose names that describe what the package provides, not what it contains +- Avoid generic names like `util`, `common`, or `base` + +#### Package Declaration Rules (CRITICAL): +- **NEVER duplicate `package` declarations** - each Go file must have exactly ONE `package` line +- When editing an existing `.go` file: + - **PRESERVE** the existing `package` declaration - do not add another one + - If you need to replace the entire file content, start with the existing package name +- When creating a new `.go` file: + - **BEFORE writing any code**, check what package name other `.go` files in the same directory use + - Use the SAME package name as existing files in that directory + - If it's a new directory, use the directory name as the package name + - Write **exactly one** `package ` line at the very top of the file +- When using file creation or replacement tools: + - **ALWAYS verify** the target file doesn't already have a `package` declaration before adding one + - If replacing file content, include only ONE `package` declaration in the new content + - **NEVER** create files with multiple `package` lines or duplicate declarations + +### Variables and Functions + +- Use mixedCaps or MixedCaps (camelCase) rather than underscores +- Keep names short but descriptive +- Use single-letter variables only for very short scopes (like loop indices) +- Exported names start with a capital letter +- Unexported names start with a lowercase letter +- Avoid using the same name for the package and a type, (e.g., avoid `http.HTTPServer`, prefer `http.Server`) + +### Interfaces + +- Name interfaces with -er suffix when possible (e.g., `Reader`, `Writer`, `Formatter`) +- Single-method interfaces should be named after the method (e.g., `Read` → `Reader`) +- Keep interfaces small and focused + +### Constants + +- Use MixedCaps for exported constants +- Use mixedCaps for unexported constants +- Group related constants using `const` blocks +- Consider using typed constants for better type safety + +## Code Style and Formatting + +### Formatting + +- Use `gofumt` to format code +- Use `goimports` to manage ordering of `import` statements +- Keep line length reasonable (no hard limit, but consider readability) +- Add blank lines to separate logical groups of code, adhering to the linter constraints. + +### Comments + +- Strive for self-documenting code; prefer clear variable names, function names, and code structure over comments +- Write comments only when necessary to explain complex implementation or non-obvious behavior +- Write comments in complete sentences in English +- Start sentences with the name of the item being described +- Package comments should start with "Package [name]" +- Use line comments (`//`) for most comments +- Document the meaning of structs, interfaces and fields and their use. +- Document the invariants expected when calling a function, the change of state + if any, and the expected state invariant when a function returns. +- Document the rationale (why) and not how it is done, unless the implementation is complex + +## Architecture and Project Structure + +### Package Organization + +- Follow standard Go project layout conventions +- Use `internal/` for packages that shouldn't be imported by external projects +- Group related functionality into packages +- Avoid circular dependencies +- Put reusable packages in `internal/` if possible + +### Dependency Management + +- Use Go modules (`go.mod` and `go.sum`) +- Keep dependencies minimal +- Regularly update dependencies for security patches +- Use `go mod tidy` to clean up unused dependencies + +## Type Safety and Language Features + +### Type Definitions + +- Define types to add meaning and type safety +- Use struct tags for JSON, XML, database mappings +- Use lower camelCase for field names in JSON tags +- Prefer explicit type conversions +- Use type assertions carefully and check whether the assertion succeeds using the second return value +- Prefer generics over unconstrained types; when an unconstrained type is truly needed, use the predeclared alias `any` instead of `interface{}` + +### Pointers vs Values Parameter + +- Use pointer receivers for large structs or when you need to modify the receiver +- Use value receivers for small structs and when immutability is desired +- Use pointer parameters when you need to modify the argument or for large structs +- Use value parameters for small structs and when you want to prevent modification +- Be consistent with the receiver type, either use pointer receivers or value receivers for a given receiver type +- Consider the zero value when choosing pointer vs value receivers + +### Interfaces and Composition + +- Accept interfaces, return concrete types +- Keep interfaces small (1-3 methods is ideal) +- Use embedding for composition +- Define interfaces close to where they're used, not where they're implemented +- Don't export interfaces unless necessary + +## Concurrency + +### Goroutines + +- Avoid creating goroutines in libraries; prefer letting the caller control concurrency +- If you must create goroutines in libraries, provide clear documentation and cleanup mechanisms +- Always know how a goroutine will exit +- Use `sync.WaitGroup` or channels to wait for goroutines +- Avoid goroutine leaks by ensuring cleanup + +### Channels + +- Use channels to communicate between goroutines +- Don't communicate by sharing memory; share memory by communicating +- Close channels from the sender side, not the receiver +- Use buffered channels when you know the capacity +- Use `select` for non-blocking operations + +### Synchronization + +- Use `sync.Mutex` for protecting shared state +- Keep critical sections small +- Use `sync.RWMutex` when you have many readers +- Choose between channels and mutexes based on the use case: use channels for communication, mutexes for protecting state +- Use `sync.Once` for one-time initialization + +## Error Handling Patterns + +### Creating Errors + +- Use `errors.New` for simple static errors (constant-like error values) +- Create custom error types for domain-specific errors +- Export error variables for sentinel errors +- Use `errors.Is` and `errors.As` for error checking + +### Error Propagation + +- Add context when propagating errors up the stack +- Use descriptive error messages with relevant context fields +- Don't log and return errors (choose one) +- Handle errors at the appropriate level +- Use structured errors with fields for better debugging and monitoring + +### Error Handling + +- Check errors immediately after the function call +- Don't ignore errors using `_` unless you have a valid reason (explain and document why) +- Preserve error chains to maintain full context, wrap errors with context using `errors.Wrap()` + +- Create custom error types when checking for specific errors is needed +- Place error returns as the last return value +- Name error variables `err` +- Keep error messages lowercase and don't end with punctuation + +### Error Lists and Multiple Errors + +- Use `errors.Join()` for collecting multiple errors (nil-safe) +- Handle validation scenarios with error accumulation + +## API Design + +### JSON APIs + +- Use struct tags to control JSON marshaling +- Validate input data +- Use pointers for optional fields +- Consider using `json.RawMessage` for delayed parsing +- Handle JSON errors appropriately + +## Performance Optimization + +### Memory Management + +- Minimize allocations in hot paths +- Reuse objects when a large number of those are allocated (consider `sync.Pool`) +- Use value receivers for small structs +- Preallocate slices when size is known +- Avoid unnecessary string-byte conversions + +### I/O: Readers and Buffers + +- Most `io.Reader` streams are consumable once; reading advances state. Do not assume a reader can be re-read without special handling +- If you must read data multiple times, buffer it once and recreate readers on demand: + - Use `io.ReadAll` (or a limited read) to obtain `[]byte`, then create fresh readers via `bytes.NewReader(buf)` or `bytes.NewBuffer(buf)` for each reuse + - For strings, use `strings.NewReader(s)`; you can `Seek(0, io.SeekStart)` on `*bytes.Reader` to rewind +- For HTTP requests, do not reuse a consumed `req.Body`. Instead: + - Keep the original payload as `[]byte` and set `req.Body = io.NopCloser(bytes.NewReader(buf))` before each send + - Prefer configuring `req.GetBody` so the transport can recreate the body for redirects/retries: `req.GetBody = func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(buf)), nil }` +- To duplicate a stream while reading, use `io.TeeReader` (copy to a buffer while passing through) or write to multiple sinks with `io.MultiWriter` +- Reusing buffered readers: call `(*bufio.Reader).Reset(r)` to attach to a new underlying reader; do not expect it to “rewind” unless the source supports seeking +- For large payloads, avoid unbounded buffering; consider streaming, `io.LimitReader`, or on-disk temporary storage to control memory + +- Use `io.Pipe` to stream without buffering the whole payload: + - Write to `*io.PipeWriter` in a separate goroutine while the reader consumes + - Always close the writer; use `CloseWithError(err)` on failures + - `io.Pipe` is for streaming, not rewinding or making readers reusable + +- **Warning:** When using `io.Pipe` (especially with multipart writers), all writes must be performed in strict, sequential order. Do not write concurrently or out of order—multipart boundaries and chunk order must be preserved. Out-of-order or parallel writes can corrupt the stream and result in errors. + +- Streaming multipart/form-data with `io.Pipe`: + - `pr, pw := io.Pipe()`; `mw := multipart.NewWriter(pw)`; use `pr` as the HTTP request body + - Set `Content-Type` to `mw.FormDataContentType()` + - In a goroutine: write all parts to `mw` in the correct order; on error `pw.CloseWithError(err)`; on success `mw.Close()` then `pw.Close()` + - Do not store request/in-flight form state on a long-lived client; build per call + - Streamed bodies are not rewindable; for retries/redirects, buffer small payloads or provide `GetBody` + + +### Profiling + +- Use built-in profiling tools (`pprof`) +- Benchmark critical code paths +- Profile before optimizing +- Focus on algorithmic improvements first +- Consider using `testing.B` for benchmarks + +## Testing + +### Test Organization + +- Keep tests in the same package (white-box testing) +- Use `_test` package suffix for black-box testing +- Name test files with `_test.go` suffix +- Place test files next to the code they test + +### Writing Tests + +- Use table-driven tests for multiple test cases +- Name tests descriptively using `Test_functionName_scenario` +- Use subtests with `t.Run` for better organization +- Test both success and error cases +- Have separate top-level tests for the success and error cases +- Use `stretchr/testify/require` package for checking expected results + +### Test Helpers + +- Mark helper functions with `t.Helper()` +- Create test fixtures for complex setup +- Use `testing.TB` interface for functions used in tests and benchmarks +- Clean up resources using `t.Cleanup()` + +## Security Best Practices + +### Input Validation + +- Validate all external input +- Use strong typing to prevent invalid states +- Sanitize data before using in SQL queries +- Be careful with file paths from user input +- Validate and escape data for different contexts (HTML, SQL, shell) + +### Cryptography + +- Use standard library crypto packages +- Use crypto/rand for random number generation +- Use TLS for network communication +- Never store plain-text passwords +- Store password hashes using functions designed for password hashing, such as PBKDF2 and scrypt + +## Documentation + +### Code Documentation + +- Prioritize self-documenting code through clear naming and structure +- Document all exported symbols with clear, concise explanations +- Start documentation with the symbol name +- Write documentation in English +- Use examples in documentation when helpful +- Keep documentation close to code +- Update documentation when code changes +- Avoid emoji in documentation and comments + +### README and Documentation Files + +- Include clear setup instructions +- Document dependencies and requirements +- Provide usage examples +- Document configuration options +- Include troubleshooting section + +## Tools and Development Workflow + +### Essential Tools + +- `golangci-lint`: Primary linter +- `go vet`: Find suspicious constructs +- `go test`: Run tests +- `go mod`: Manage dependencies +- `go generate`: Code generation + +### Development Practices + +- Run tests before committing +- Keep commits focused and atomic +- Write meaningful commit messages +- Review diffs before committing + +## Common Pitfalls to Avoid + +- Not checking errors +- Ignoring race conditions +- Creating goroutine leaks +- Not using defer for cleanup +- Modifying maps concurrently +- Not understanding nil interfaces vs nil pointers +- Forgetting to close or release resources (files, connections) +- Using global variables unnecessarily +- Over-using unconstrained types (e.g., `any`); prefer specific types or generic type parameters with constraints. If an unconstrained type is required, use `any` rather than `interface{}` +- Not considering the zero value of types +- **Creating duplicate `package` declarations** - this is a compile error; always check existing files before adding package declarations