|
| 1 | +# Coding Style Guide |
| 2 | + |
| 3 | +This style guide defines the conventions for contributing code and tests across all IronCore projects. |
| 4 | +Following these standards ensures consistency and helps maintainers review contributions efficiently. |
| 5 | + |
| 6 | +## Code Style |
| 7 | + |
| 8 | +### Go Conventions |
| 9 | + |
| 10 | +All IronCore projects are written in Go. We follow the standard Go style guides as our baseline: |
| 11 | + |
| 12 | +- [Effective Go](https://go.dev/doc/effective_go) — the foundational language guide. |
| 13 | +- [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments) — the de facto standard for code reviews. |
| 14 | + |
| 15 | +The rules below supplement these upstream guides with IronCore-specific conventions. Do not duplicate what is already |
| 16 | +covered upstream — read those guides first. |
| 17 | + |
| 18 | +### Formatting |
| 19 | + |
| 20 | +- Run `gofmt` (or `goimports`) on all code before committing. This is non-negotiable. |
| 21 | +- Use your editor or a pre-commit hook to automate formatting. |
| 22 | + |
| 23 | +### Naming |
| 24 | + |
| 25 | +- Follow Go naming conventions: `MixedCaps` for exported identifiers, `mixedCaps` for unexported ones. Never use |
| 26 | + underscores in Go names. |
| 27 | +- Keep initialisms consistent: `URL` not `Url`, `ID` not `Id`, `HTTP` not `Http`. |
| 28 | +- Use descriptive package names. Avoid generic names like `util`, `common`, `helpers`, or `misc`. |
| 29 | +- Controller and reconciler types should clearly indicate the resource they manage |
| 30 | + (e.g., `MachineReconciler`, `VolumeController`). |
| 31 | + |
| 32 | +### Imports |
| 33 | + |
| 34 | +Organize imports into three groups separated by blank lines: |
| 35 | + |
| 36 | +```go |
| 37 | +import ( |
| 38 | + // Standard library |
| 39 | + "context" |
| 40 | + "fmt" |
| 41 | + |
| 42 | + // Third-party packages |
| 43 | + "sigs.k8s.io/controller-runtime/pkg/client" |
| 44 | + |
| 45 | + // IronCore packages |
| 46 | + "github.com/ironcore-dev/ironcore/api/compute/v1alpha1" |
| 47 | +) |
| 48 | +``` |
| 49 | + |
| 50 | +Avoid renaming imports unless there is a name collision. |
| 51 | + |
| 52 | +### Error Handling |
| 53 | + |
| 54 | +- Never discard errors with `_`. Handle every error or explicitly document why it is safe to ignore. |
| 55 | +- Return errors as the last return value. Handle errors early and return — keep the happy path at minimal indentation. |
| 56 | +- Use lowercase error strings without trailing punctuation: `fmt.Errorf("failed to create machine")`. |
| 57 | +- Wrap errors with context: `fmt.Errorf("reconciling machine %s: %w", name, err)`. |
| 58 | + |
| 59 | +### Comments |
| 60 | + |
| 61 | +- Write comments that explain *why*, not *what*. The code itself shows what it does. |
| 62 | +- Comments on exported identifiers become godoc — start with the identifier name and write complete sentences. |
| 63 | +- If reviewers ask questions about why code is written a certain way, that is a sign that a comment would help. |
| 64 | + |
| 65 | +### Logging |
| 66 | + |
| 67 | +- Use structured logging consistently. Prefer key-value pairs over formatted strings. |
| 68 | +- Use appropriate log levels: |
| 69 | + - **Error** — the reconciler cannot proceed; requires attention. |
| 70 | + - **Info** — significant events (resource created, deleted, reconciled). |
| 71 | + - **Debug/V(1)** — detailed information useful for troubleshooting. |
| 72 | + |
| 73 | +### Linting |
| 74 | + |
| 75 | +All projects should use [golangci-lint](https://golangci-lint.run/) with the project's `.golangci.yml` configuration. |
| 76 | +Run linters locally before pushing: |
| 77 | + |
| 78 | +```shell |
| 79 | +make lint |
| 80 | +``` |
| 81 | + |
| 82 | +## Testing |
| 83 | + |
| 84 | +Good tests are as important as good code. Tests protect against regressions, document expected behavior, and give |
| 85 | +reviewers confidence that changes work correctly. |
| 86 | + |
| 87 | +### Testing Pyramid |
| 88 | + |
| 89 | +IronCore projects use three levels of testing: |
| 90 | + |
| 91 | +| Level | Purpose | Tools | |
| 92 | +| --- | --- | --- | |
| 93 | +| **Unit tests** | Test individual functions and methods in isolation. | Go standard `testing` package | |
| 94 | +| **Integration tests** | Test controllers against a real API server using envtest. | [controller-runtime envtest](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/envtest) | |
| 95 | +| **End-to-end tests** | Test the full system with real components deployed. | [Ginkgo](https://onsi.github.io/ginkgo/) + [Gomega](https://onsi.github.io/gomega/) | |
| 96 | + |
| 97 | +### Writing Tests |
| 98 | + |
| 99 | +- **Table-driven tests** are the preferred pattern for unit tests with multiple input/output cases: |
| 100 | + |
| 101 | +```go |
| 102 | +func TestValidateMachineName(t *testing.T) { |
| 103 | + tests := []struct { |
| 104 | + name string |
| 105 | + input string |
| 106 | + wantErr bool |
| 107 | + }{ |
| 108 | + {name: "valid name", input: "my-machine", wantErr: false}, |
| 109 | + {name: "empty name", input: "", wantErr: true}, |
| 110 | + {name: "name with spaces", input: "my machine", wantErr: true}, |
| 111 | + } |
| 112 | + for _, tt := range tests { |
| 113 | + t.Run(tt.name, func(t *testing.T) { |
| 114 | + err := ValidateMachineName(tt.input) |
| 115 | + if (err != nil) != tt.wantErr { |
| 116 | + t.Errorf("ValidateMachineName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) |
| 117 | + } |
| 118 | + }) |
| 119 | + } |
| 120 | +} |
| 121 | +``` |
| 122 | + |
| 123 | +- **Test names** should be descriptive and specific. Use the pattern `Test<Function>_<scenario>` for unit tests. |
| 124 | +- **Assertions** in Ginkgo/Gomega tests should use matchers, not boolean checks: |
| 125 | + |
| 126 | +| Do | Don't | |
| 127 | +| --- | --- | |
| 128 | +| `Expect(pod.Status.Phase).To(Equal(corev1.PodRunning))` | `Expect(pod.Status.Phase == corev1.PodRunning).To(BeTrue())` | |
| 129 | + |
| 130 | +- **Failure messages** must be actionable. Include what was expected, what was received, and enough context to debug |
| 131 | + without re-running: |
| 132 | + |
| 133 | +| Do | Don't | |
| 134 | +| --- | --- | |
| 135 | +| `"expected Machine %s to be in Running phase, got %s"` | `"wrong phase"` | |
| 136 | + |
| 137 | +### Test Reliability |
| 138 | + |
| 139 | +- Tests must be deterministic. A test that passes 99% of the time is unreliable in CI. |
| 140 | +- Use `gomega.Eventually` for polling conditions instead of `time.Sleep`. |
| 141 | +- Clean up resources after tests. Use `DeferCleanup` in Ginkgo or `t.Cleanup` in standard tests. |
| 142 | +- Keep tests fast. Unit tests should complete in seconds; integration tests in under two minutes. |
| 143 | + |
| 144 | +### Running Tests |
| 145 | + |
| 146 | +Before submitting a pull request, run the full test suite locally: |
| 147 | + |
| 148 | +```shell |
| 149 | +make test # Unit and integration tests |
| 150 | +make lint # Linter checks |
| 151 | +make verify # Code generation and manifests are up to date |
| 152 | +``` |
| 153 | + |
| 154 | +## Commit Messages |
| 155 | + |
| 156 | +- **Subject line**: imperative mood, max 72 characters, capitalize the first word, no trailing period. |
| 157 | +- **Body**: wrap at 72 characters, explain *what* and *why*, separate from the subject with a blank line. |
| 158 | +- Reference related issues: `Fixes #123` or `Relates to #456`. |
| 159 | + |
| 160 | +| Do | Don't | |
| 161 | +| --- | --- | |
| 162 | +| `Add volume snapshot support` | `Added volume snapshot support` | |
| 163 | +| `Fix nil pointer in machine reconciler` | `fixed stuff` | |
| 164 | + |
| 165 | +## Pull Requests |
| 166 | + |
| 167 | +- Keep pull requests small and focused — one logical change per PR. |
| 168 | +- Separate bug fixes from refactoring from new features. |
| 169 | +- Include tests for any new functionality or bug fix. |
| 170 | +- Ensure all CI checks pass before requesting review. |
| 171 | +- Write a clear PR description explaining what changed and why. |
0 commit comments