|
| 1 | +--- |
| 2 | +title: "Coding Practices" |
| 3 | +linkTitle: "Practices" |
| 4 | +weight: 5 |
| 5 | +description: "Cross-cutting coding and git practices for all DevRail projects." |
| 6 | +--- |
| 7 | + |
| 8 | +These practices apply to every DevRail project regardless of language. They are enforced by code review, not automated tooling. The per-language pages cover *which tools run*; this page covers *how to write the code those tools check*. |
| 9 | + |
| 10 | +## Principles |
| 11 | + |
| 12 | +| Principle | Rule | |
| 13 | +|---|---| |
| 14 | +| **DRY** (Don't Repeat Yourself) | Extract repeated logic into shared functions or modules. If the same block appears three or more times, refactor it. | |
| 15 | +| **KISS** (Keep It Simple, Stupid) | Choose the simplest solution that meets the requirement. Avoid clever code that requires comments to explain. | |
| 16 | +| **YAGNI** (You Aren't Gonna Need It) | Do not build for hypothetical future requirements. Implement what is needed now. | |
| 17 | +| **Single Responsibility** | Each function, class, or module does one thing. If a description requires "and", split it. | |
| 18 | +| **Separation of Concerns** | Keep business logic, data access, configuration, and presentation in distinct layers. | |
| 19 | +| **Fail Fast** | Detect errors as early as possible. Validate inputs at boundaries. Return or raise immediately on invalid state. | |
| 20 | +| **Least Surprise** | Code should behave as a reader would expect. Follow language idioms and project conventions. | |
| 21 | +| **Idempotency** | Operations must be safe to re-run. The result of running something once must be identical to running it N times. | |
| 22 | + |
| 23 | +## Idempotency |
| 24 | + |
| 25 | +Every script, migration, deployment, and configuration change must be safe to re-run without causing damage or duplication. |
| 26 | + |
| 27 | +### Patterns by Context |
| 28 | + |
| 29 | +| Context | Pattern | |
| 30 | +|---|---| |
| 31 | +| **Shell scripts** | `command -v tool \|\| install_tool`, `mkdir -p`, guard file writes with existence checks. | |
| 32 | +| **Database migrations** | Use a migration framework that tracks applied migrations by ID. Never rely on manual execution order. | |
| 33 | +| **Terraform** | Declare desired state, not imperative steps. `terraform apply` is inherently idempotent when state is managed correctly. | |
| 34 | +| **Ansible** | Use declarative modules over `command`/`shell`. Set `creates:` or `removes:` on shell tasks. Use `changed_when` to avoid false changes. | |
| 35 | +| **CI/CD pipelines** | Pipeline stages must not fail or produce different results when re-triggered. Use `--if-not-exists` flags and check artifact existence before publishing. | |
| 36 | +| **Python** | Guard resource creation with existence checks. Use `os.makedirs(path, exist_ok=True)`, catch `FileExistsError`. | |
| 37 | +| **Configuration management** | Write config files atomically (write to temp, then rename). Check current state before modifying. Never append without checking for existing entries. | |
| 38 | + |
| 39 | +### Anti-Patterns |
| 40 | + |
| 41 | +- Blindly appending to files (duplicate entries on re-run) |
| 42 | +- `INSERT` without `ON CONFLICT` / `IF NOT EXISTS` |
| 43 | +- Scripts that assume clean state (empty directory, fresh database) |
| 44 | +- Provisioners or setup scripts with no guards |
| 45 | + |
| 46 | +## Error Handling |
| 47 | + |
| 48 | +1. **Validate inputs at system boundaries.** User input, API payloads, environment variables, and file contents must be validated before use. Internal function-to-function calls can trust types and contracts. |
| 49 | +2. **No swallowed exceptions.** Every `except`, `catch`, or error branch must either handle the error meaningfully or re-raise/propagate it. Bare `except:` (Python) and empty `catch {}` blocks are prohibited. |
| 50 | +3. **Fail with meaningful messages.** Error messages must include what went wrong, what was expected, and (when possible) how to fix it. |
| 51 | +4. **Use language-appropriate error patterns.** See the per-language standard for specifics (e.g., `set -euo pipefail` in Bash, specific exception types in Python). |
| 52 | +5. **Log errors before propagating.** Use the appropriate logging mechanism so errors are visible in structured output even if the caller catches and handles them. |
| 53 | + |
| 54 | +## Testing |
| 55 | + |
| 56 | +### Test Pyramid |
| 57 | + |
| 58 | +Maintain a healthy ratio: **unit tests > integration tests > end-to-end tests**. |
| 59 | + |
| 60 | +- **Unit tests** -- fast, isolated, test a single function or method. Mock external dependencies. |
| 61 | +- **Integration tests** -- verify that components work together (e.g., script + filesystem, module + provider). |
| 62 | +- **End-to-end tests** -- validate full workflows. Fewer of these; they are slower and more brittle. |
| 63 | + |
| 64 | +### Test Naming |
| 65 | + |
| 66 | +Use descriptive names: `test_<what>_<condition>_<expected>`. |
| 67 | + |
| 68 | +### Coverage |
| 69 | + |
| 70 | +- Aim for meaningful coverage, not vanity metrics. 80% coverage of critical paths beats 100% coverage padded with trivial assertions. |
| 71 | +- **New code must include tests.** PRs that add logic without tests are incomplete. |
| 72 | + |
| 73 | +## Git Workflow |
| 74 | + |
| 75 | +- **Never push directly to `main`.** All changes reach the default branch through a pull/merge request. |
| 76 | +- **Branch naming:** `type/short-description` (e.g., `feat/add-ansible-support`, `fix/shellcheck-false-positive`). |
| 77 | +- **Conventional commits:** Every commit message follows `type(scope): description`. |
| 78 | +- **Minimum 1 approval required** before merging. No self-merge (exception: solo maintainers after CI passes). |
| 79 | +- **Never force-push shared branches.** Force push is acceptable on your own feature branches only. |
| 80 | +- **Squash-merge feature branches** into `main` for clean, linear history. |
| 81 | +- **No secrets in commits.** Enforced by `gitleaks` in pre-commit hooks and `make scan`. |
| 82 | + |
| 83 | +## Code Organization |
| 84 | + |
| 85 | +- **Function length: ~50 lines maximum.** If a function exceeds this, consider splitting it. |
| 86 | +- **One primary concern per file.** A file named `auth.py` should contain authentication logic, not unrelated utilities. |
| 87 | +- **No circular dependencies.** If A imports B and B imports A, refactor to extract the shared dependency into C. |
| 88 | + |
| 89 | +## Dependencies |
| 90 | + |
| 91 | +- **Lock files are mandatory.** Use the appropriate lock file for each language and commit it to version control. |
| 92 | +- **Pin versions.** Allow compatible ranges only in dependency declarations; lock files pin to exact versions. |
| 93 | +- **Respond to security advisories promptly.** Run `make security` and `make scan` after dependency updates. |
| 94 | + |
| 95 | +## Notes |
| 96 | + |
| 97 | +- These practices are enforced by code review, not by automated tooling. Automated enforcement is covered by the per-language tool standards. |
| 98 | +- When a practice here conflicts with a language-specific standard, the language-specific standard takes precedence. |
| 99 | +- These practices are embedded in every project's DEVELOPMENT.md via the DevRail template repos. |
| 100 | +- Full reference: [Coding Practices](https://gitlab.mfsoho.linkridge.net/OrgDocs/development-standards/-/blob/main/standards/coding-practices.md) and [Git Workflow](https://gitlab.mfsoho.linkridge.net/OrgDocs/development-standards/-/blob/main/standards/git-workflow.md) in the planning repo. |
0 commit comments