|
| 1 | +# Base Development Rules |
| 2 | + |
| 3 | +**Purpose:** Core development principles and standards that apply to all projects. |
| 4 | + |
| 5 | +**Scope:** Universal rules - copy this file to all your projects and keep it consistent. |
| 6 | + |
| 7 | +## Philosophy |
| 8 | + |
| 9 | +- **Simplicity is key**. Simple solutions over clever ones. |
| 10 | +- **Standards matter**. Follow established conventions. |
| 11 | +- **Think before implementing**. Plan, then code. |
| 12 | +- **Baby steps**. Work incrementally, one logical change at a time. |
| 13 | +- **Iterate**. Ship small, learn, improve. |
| 14 | +- **Delete bad code**. Refactor without hesitation. |
| 15 | +- **Don't build what isn't needed**. Question every feature. |
| 16 | +- **Question assumptions**. Ask when unclear, challenge requirements that seem wrong. |
| 17 | +- **Testing is mandatory**. Code without tests is incomplete. |
| 18 | +- **Maintainability > features**. Code will be read more than written. |
| 19 | +- **Leave code better than you found it**. Fix code smells, clean up ruthlessly. |
| 20 | +- **No breadcrumbs**. Delete code completely, no comments about relocations. |
| 21 | + |
| 22 | +## Language & Tools |
| 23 | + |
| 24 | +### Documentation search |
| 25 | + |
| 26 | +- Always use Context7 MCP when I need library/API documentation, code generation, setup or configuration steps without me having to explicitly ask. |
| 27 | + |
| 28 | + |
| 29 | +### Go |
| 30 | + |
| 31 | +#### Style |
| 32 | + |
| 33 | +- Follow standard Go conventions (gofmt, golint, go vet) |
| 34 | +- Fully typed code - types as documentation |
| 35 | +- Use standard library when possible |
| 36 | +- Idiomatic Go: simple error handling, interfaces, composition |
| 37 | +- Keep packages small and focused |
| 38 | +- No magic, no frameworks unless necessary |
| 39 | +- Self-documenting code: clear names over comments |
| 40 | + |
| 41 | +#### Dependency Injection |
| 42 | + |
| 43 | +Use config structs for explicit dependency injection with validation: |
| 44 | + |
| 45 | +```go |
| 46 | +type ServiceConfig struct { |
| 47 | + Repository Repository |
| 48 | + Logger Logger |
| 49 | + Timeout time.Duration |
| 50 | +} |
| 51 | + |
| 52 | +func (c *ServiceConfig) defaults() error { |
| 53 | + if c.Repository == nil { |
| 54 | + return fmt.Errorf("repository is required") |
| 55 | + } |
| 56 | + if c.Logger == nil { |
| 57 | + c.Logger = log.Noop // Optional: provide defaults |
| 58 | + } |
| 59 | + if c.Timeout == 0 { |
| 60 | + c.Timeout = 30 * time.Second |
| 61 | + } |
| 62 | + return nil |
| 63 | +} |
| 64 | + |
| 65 | +func NewService(cfg ServiceConfig) (*Service, error) { |
| 66 | + if err := cfg.defaults(); err != nil { |
| 67 | + return nil, fmt.Errorf("invalid config: %w", err) |
| 68 | + } |
| 69 | + return &Service{repo: cfg.Repository, logger: cfg.Logger, timeout: cfg.Timeout}, nil |
| 70 | +} |
| 71 | +``` |
| 72 | + |
| 73 | +- Explicit dependencies, no globals |
| 74 | +- Validate required deps, default optional ones |
| 75 | +- Return typed errors from constructors |
| 76 | + |
| 77 | +#### Checking and feedback loops |
| 78 | + |
| 79 | +- Use `go run` instead of `go build` as much as possible. |
| 80 | +- Use `go run` with fakes (--*fake* flags. Use `--help` for discovery) for automatic iteration. |
| 81 | + |
| 82 | +#### Testing |
| 83 | + |
| 84 | +- Table testing uses a `map[string]struct{...` where the key is the description identifying the test. |
| 85 | +- For mocks and complex tests data preparations, use functions instead of test field. |
| 86 | +- Use mockery for tests (`.mockery.yml` at project root) |
| 87 | +- Mock packages follow naming: `{package}mock/mocks.go` |
| 88 | +- To add a new mock: |
| 89 | + 1. Add interface to `.mockery.yml` under `packages:` |
| 90 | + 2. Run `make go-gen` (or `mockery` directly) |
| 91 | + 3. Import as `"path/to/package/packagemock"` |
| 92 | + |
| 93 | +Example: |
| 94 | + |
| 95 | +```go |
| 96 | +func TestAddServiceSecretGroup(t *testing.T) { |
| 97 | + tests := map[string]struct { |
| 98 | + mock func(mc *vaultmock.Client) |
| 99 | + secret model.SecretGroup |
| 100 | + serviceName string |
| 101 | + serviceEnv string |
| 102 | + expErr bool |
| 103 | + }{ |
| 104 | + "Having an error while adding a secret it should fail.": { |
| 105 | + secret: model.SecretGroup{Data: map[string]any{"k1": "v1", "k2": "v2"}}, |
| 106 | + mock: func(mc *vaultmock.Client) { |
| 107 | + mc.On("KV2Patch", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Once().Return(nil, fmt.Errorf("something")) |
| 108 | + }, |
| 109 | + serviceName: "test-svc", |
| 110 | + serviceEnv: "test-env", |
| 111 | + expErr: true, |
| 112 | + }, |
| 113 | + |
| 114 | + "Adding secrets data on a existing secret in Vault shouldn't fail.": { |
| 115 | + secret: model.SecretGroup{Data: map[string]any{"k1": "v1", "k2": "v2"}}, |
| 116 | + mock: func(mc *vaultmock.Client) { |
| 117 | + expData := map[string]any{"k1": "v1", "k2": "v2"} |
| 118 | + mc.On("KV2Patch", mock.Anything, "fit", "services/test-svc-test-env/default", expData, mock.AnythingOfType("[]api.KVOption")).Once().Return(nil, nil) |
| 119 | + }, |
| 120 | + serviceName: "test-svc", |
| 121 | + serviceEnv: "test-env", |
| 122 | + }, |
| 123 | + |
| 124 | + "Adding secrets data on a missing secret in Vault shouldn't fail.": { |
| 125 | + secret: model.SecretGroup{Data: map[string]any{"k1": "v1", "k2": "v2"}}, |
| 126 | + mock: func(mc *vaultmock.Client) { |
| 127 | + expData := map[string]any{"k1": "v1", "k2": "v2"} |
| 128 | + mc.On("KV2Patch", mock.Anything, "fit", "services/test-svc-test-env/default", expData, mock.AnythingOfType("[]api.KVOption")).Once().Return(nil, vaultapi.ErrSecretNotFound) |
| 129 | + mc.On("KV2Put", mock.Anything, "fit", "services/test-svc-test-env/default", expData).Once().Return(nil, nil) |
| 130 | + }, |
| 131 | + serviceName: "test-svc", |
| 132 | + serviceEnv: "test-env", |
| 133 | + }, |
| 134 | + } |
| 135 | + |
| 136 | + for name, test := range tests { |
| 137 | + t.Run(name, func(t *testing.T) { |
| 138 | + assert := assert.New(t) |
| 139 | + require := require.New(t) |
| 140 | + |
| 141 | + mc := &vaultmock.Client{} |
| 142 | + test.mock(mc) |
| 143 | + |
| 144 | + repo, err := vault.NewRepository(mc, log.Noop, vault.NoopEventRecorder{}) |
| 145 | + require.NoError(err) |
| 146 | + |
| 147 | + err = repo.AddServiceSecretGroup(context.TODO(), test.serviceName, test.serviceEnv, test.secret) |
| 148 | + |
| 149 | + if test.expErr { |
| 150 | + assert.Error(err) |
| 151 | + } else { |
| 152 | + assert.NoError(err) |
| 153 | + } |
| 154 | + |
| 155 | + mc.AssertExpectations(t) |
| 156 | + }) |
| 157 | + } |
| 158 | +} |
| 159 | +``` |
| 160 | + |
| 161 | +#### Function calls |
| 162 | + |
| 163 | +- Having a context (`ctx context.Context`) as first argument on all public functions. |
| 164 | + |
| 165 | +#### Generated Code |
| 166 | + |
| 167 | +- Never edit generated files directly - edit the source, then regenerate |
| 168 | +- Mark generated directories clearly in `project.md` (e.g., `pkg/*/gen/`, `*/_gen/`) |
| 169 | +- Common generation triggers: |
| 170 | + - Interface changes → regenerate mocks (`make go-gen`) |
| 171 | + - API/CRD type changes → regenerate clients |
| 172 | + - Proto/OpenAPI changes → regenerate stubs |
| 173 | +- Run `make gen` (or equivalent) after source changes |
| 174 | + |
| 175 | +### Bash |
| 176 | +- POSIX-compliant when possible |
| 177 | +- Use `set -euo pipefail` for scripts |
| 178 | +- Fail fast, explicit error handling |
| 179 | +- Keep scripts simple and readable |
| 180 | +- Clear variable names |
| 181 | + |
| 182 | +### Tooling |
| 183 | +- Simple tooling: use a few tools, master them |
| 184 | +- Prefer Makefile for consistency |
| 185 | +- Use `go run ...` commands for developing and checking (instead of `go build` + exec binary). |
| 186 | +- Use plain git commands, no custom tooling for git management |
| 187 | +- Search official docs when stuck, don't pivot without understanding |
| 188 | + |
| 189 | +## Testing |
| 190 | + |
| 191 | +- Write tests first or alongside code |
| 192 | +- Unit tests are mandatory for business logic |
| 193 | +- Integration tests for critical paths |
| 194 | +- Tests must be fast and deterministic |
| 195 | +- Use table-driven tests in Go |
| 196 | +- Mock external dependencies |
| 197 | +- Test coverage matters, but 100% isn't the goal |
| 198 | +- If you need a browser to check, test or develop, use the chrome-devtools-mcp MCP |
| 199 | + |
| 200 | +### Integration Tests |
| 201 | + |
| 202 | +- Integration tests are for CI, not for local agent execution |
| 203 | +- Agents should NOT run integration tests unless explicitly requested by the user |
| 204 | +- If integration test results are needed, check CI outputs instead |
| 205 | +- This ensures sandboxing - agents should not connect to external systems (databases, K8s clusters, APIs) |
| 206 | +- Integration tests are typically gated by environment variables (document these in `project.md`) |
| 207 | + |
| 208 | +### Pre-Commit Validation |
| 209 | +- Run tests and checks before committing |
| 210 | +- Fix issues before commit, not after |
| 211 | +- Use automated checks (linters, formatters, tests) |
| 212 | +- Never commit broken code |
| 213 | + |
| 214 | + |
| 215 | +## Git & GitHub Workflow |
| 216 | + |
| 217 | +### Branching |
| 218 | +- **Never commit directly to main/master/default branch** |
| 219 | +- Always create a branch: `{username}/branch-name` |
| 220 | +- Branch names: lowercase, dashes, numbers, letters only (no spaces) |
| 221 | +- Keep branch names small and concise (max 30 chars if possible) |
| 222 | +- Example: `slok/fix-auth-bug`, `slok/add-metrics` |
| 223 | + |
| 224 | +### Commits |
| 225 | +- Prefer small commit history (1 commit is ideal) |
| 226 | +- Don't need to rebase always, but keep history clean |
| 227 | +- Commit messages: single phrase describing the intent |
| 228 | +- Complex commits can have longer descriptions when needed |
| 229 | +- Ensure commits are signed (SSH signing configured) |
| 230 | +- Standard workflow: `git add` then `git commit -svm "message"` |
| 231 | + |
| 232 | +### Pull Requests |
| 233 | +- Work in branches, merge via PRs |
| 234 | +- Keep PRs focused and small (baby steps) |
| 235 | +- Run unit tests before creating PR (integration tests run in CI) |
| 236 | + |
| 237 | +**PR Title:** |
| 238 | +- Concise, descriptive phrase (similar to commit message style) |
| 239 | +- Describe the intent, not implementation details |
| 240 | + |
| 241 | +**PR Description:** |
| 242 | +- Brief summary: few bullet points max |
| 243 | +- Only call out TODOs or tradeoffs if relevant |
| 244 | +- Keep it minimal - no walls of text, no file listings |
| 245 | +- If PR was generated or driven by an AI agent/LLM, add an "AI" section at the end with: |
| 246 | + - Tool/model used (e.g., Claude, GPT-4, Copilot) |
| 247 | + - Involvement level: "generated" (fully AI) or "assisted" (human-driven with AI help) |
| 248 | + |
| 249 | +**Before creating PR:** |
| 250 | +- Run unit tests locally |
| 251 | +- Review your own changes first |
| 252 | +- Ensure commit history is clean (prefer 1 commit) |
| 253 | +- CI will handle integration tests |
| 254 | + |
| 255 | +## Communication |
| 256 | + |
| 257 | +- Technical and concise |
| 258 | +- No walls of text |
| 259 | +- Show code examples |
| 260 | +- Reference file:line for context when discussing code during development |
| 261 | +- Ask when unclear, don't assume |
| 262 | +- Verify understanding before implementing |
| 263 | + |
| 264 | +## Workflow |
| 265 | + |
| 266 | +1. **Understand** - Read existing code, understand the problem |
| 267 | +2. **Think** - Plan the approach, consider alternatives |
| 268 | +3. **Implement** - Write simple, tested code in small increments |
| 269 | +4. **Verify** - Run tests, check edge cases |
| 270 | +5. **Iterate** - Improve based on results |
| 271 | + |
| 272 | +When taking on complex work: |
| 273 | +1. Think about the architecture |
| 274 | +2. Research official docs, best practices |
| 275 | +3. Review existing codebase |
| 276 | +4. Compare research with codebase, choose best fit |
| 277 | +5. Implement or discuss tradeoffs |
| 278 | + |
| 279 | +## Code Review Mindset |
| 280 | + |
| 281 | +When reviewing or writing code, ask: |
| 282 | +- Is this the simplest solution? |
| 283 | +- Is it tested? |
| 284 | +- Is it fully typed? |
| 285 | +- Can it be maintained in 6 months? |
| 286 | +- Does it follow standards? |
| 287 | +- Are there repeated patterns that should be refactored? |
| 288 | +- Are names clear and self-documenting? |
| 289 | +- Should this code exist at all? |
| 290 | + |
| 291 | +## Error Handling |
| 292 | + |
| 293 | +- Fail fast with clear error messages |
| 294 | +- Provide context in errors |
| 295 | +- Handle errors explicitly, never silently |
| 296 | +- Better to crash early than corrupt data |
| 297 | + |
| 298 | +### Sentinel Errors |
| 299 | + |
| 300 | +Define shared sentinel errors in a central package (e.g., `internal/errors/`): |
| 301 | + |
| 302 | +```go |
| 303 | +var ( |
| 304 | + ErrNotFound = errors.New("not found") |
| 305 | + ErrNotValid = errors.New("not valid") |
| 306 | + ErrAlreadyExists = errors.New("already exists") |
| 307 | +) |
| 308 | +``` |
| 309 | + |
| 310 | +Wrap with context and sentinel for both human readability and programmatic checks: |
| 311 | + |
| 312 | +```go |
| 313 | +return fmt.Errorf("failed to get user %s: %w: %w", id, err, errors.ErrNotFound) |
| 314 | +``` |
| 315 | + |
| 316 | +This enables `errors.Is(err, ErrNotFound)` checks while preserving context. |
| 317 | + |
| 318 | +## Final Handoff |
| 319 | + |
| 320 | +Before completing a task: |
| 321 | +- Small recap (not walls of text, few lines or highlights) |
| 322 | +- Call out any TODOs or follow-up work |
| 323 | +- Flag uncertainties or tradeoffs made |
| 324 | + |
| 325 | +### Dependencies |
| 326 | + |
| 327 | +- Research well-maintained options before adding |
| 328 | +- Prefer popular, actively maintained libraries |
| 329 | +- Confirm fit and necessity with context before adding |
0 commit comments