Skip to main content

Developer Best Practices Guide

# CONTRIBUTING.md

# Contributing Guide

This repository follows strict coding, review, testing, and release standards. Every contributor (AI agent) must follow the rules below.

---

## 1) Repository Workflow

### 1.1 Always sync before starting work
Run these commands before creating or updating your branch:

```bash
git checkout main
git pull origin main
git fetch --all
```

### 1.2 One PR = One logical change
- Create a separate PR for every feature, bugfix, refactor, or documentation update.
- Do **not** mix multiple features/fixes in the same PR.
- Keep PRs focused and reviewable.

### 1.3 Reviewers
For every Python-based repository PR:
- Add the required reviewer(s)
- Add **GitHub Copilot** as reviewer

---

## 2) Branch Naming Convention

Use the following branch naming pattern:

```text
<type>/<your-name>/<short_description>
```

### Allowed branch types
- `feature/` → New functionality
- `bugfix/` → Bug fix
- `hotfix/` → Urgent production fix
- `chore/` → Maintenance, cleanup, dependency updates
- `docs/` → Documentation-only changes
- `refactor/` → Internal code improvement (no new feature, no bug fix)

### Examples
```text
feature/saksham/user_session_tracking
bugfix/sachin/oauth_refresh_issue
hotfix/rahul/payment_retry_failure
chore/priya/dependency_updates
refactor/amit/service_layer_cleanup
docs/neha/api_usage_examples
```

### Naming rules
- Use lowercase only
- Use `snake_case`
- Keep the description short and meaningful
- Include your name in the branch name

---

## 3) Coding Standards

### 3.1 General rules
- Use the **Pythonic** way of writing code
- Follow **DRY** (Don't Repeat Yourself)
- Keep logic simple and easy to understand
- Do not overcomplicate the implementation
- Prioritize readability and maintainability
- Write production-ready code

### 3.2 Software engineering principles
- Follow **SOLID** principles wherever applicable
- Write clean, modular, testable code
- Prefer composition and clear abstractions over deeply coupled logic
- Separate concerns properly (API, service, repository, utility, model, constants, etc.)

### 3.3 Object-oriented programming
- Follow **OOP** standards where appropriate
- Keep classes focused on a single responsibility
- Avoid god classes / oversized service classes
- Use inheritance only when it is truly needed

### 3.4 Design patterns
- Use the correct design pattern where applicable
- Prefer clarity over pattern overuse
- Use **Factory Pattern** for common integrations across multiple cloud/external applications

### 3.5 Asynchronous programming
- Use **asynchronous programming** (`async` / `await`) where applicable
- Do not block async workflows with unnecessary synchronous operations
- Keep async usage consistent across the call chain

---

## 4) Project Structure Rules

- Each Python class must be placed in a **separate `.py` file**
- Follow a clean and modular folder structure
- Use `snake_case` for:
  - file names
  - method names
  - variable names
  - function names
- Keep file responsibilities limited and clear
- Avoid putting unrelated classes or utilities into the same file

### Recommended separation
- `api/` → route or controller layer
- `service/` → business logic
- `repository/` → DB interaction
- `models/` → schemas/entities/domain models
- `constants/` → all static messages and constants
- `tests/` → pytest-based tests

---

## 5) Constants and Messages

- Every user-facing or system message must come from a **constants file**
- Do **not** hardcode messages directly in business logic
- Keep error messages, labels, and static responses centralized
- Reuse constants to maintain consistency across the codebase

### Not allowed
```python
raise ValueError("Invalid request")
```

### Preferred
```python
raise ValueError(ErrorMessages.INVALID_REQUEST)
```

---

## 6) Docstrings and Documentation in Code

- Every Python file must contain proper docstrings where needed
- Add docstrings for:
  - modules
  - classes
  - public methods
  - non-trivial helper functions
- Keep docstrings clear and meaningful
- Explain intent, parameters, return types, and important side effects

### Minimum expectation
- What the class/function does
- Input parameters
- Return value
- Exceptions raised (if relevant)

---

## 7) Database Rules

### 7.1 Schema change restrictions
- Do **not** create a new DB table without discussion/approval
- Do **not** create a new DB column without discussion/approval

### 7.2 Documentation requirements for DB objects
- Every DB table must include a description of what it stores / why it exists
- Every DB column must include a description of what it stores / how it is used

### 7.3 Timestamps
- Any timestamp saved in the database must be stored in **UTC**

### 7.4 How DB changes must happen
Any DB update involving the following must happen through **Alembic**:
- schema change
- CRUD-related DB update scripts
- seed data insertion
- migration for reference/master data updates when applicable

### 7.5 Alembic note
- Use Alembic for all database migration work
- Do not bypass migrations with manual DB changes in application code

---

## 8) Testing Requirements

### 8.1 Framework
- Use **pytest** for all tests
- Add proper unit tests and integration tests wherever applicable

### 8.2 Coverage
- Code coverage must be **above 90%**
- PRs below the coverage threshold should not be considered ready for merge

### 8.3 Test quality expectations
- Test actual behavior, not implementation details only
- Cover positive, negative, and edge cases
- Mock external dependencies where appropriate
- Keep tests readable and maintainable

---

## 9) Linting and Code Quality Gates

### 9.1 Pylint
- Run pylint before marking the PR ready for merge
- Required pylint score: **above 9.30**

### 9.2 Command
Run:

```bash
run_pylint.bat
```

### 9.3 Alembic exception
- Alembic-related pylint issues may be ignored where already agreed

### 9.4 Readiness rule
A PR is not ready to merge unless:
- pylint score is acceptable
- tests pass
- coverage is above threshold

---

## 10) Versioning and Release File Updates

For **every PR**, update the following files if the repository/version policy requires it:
- `application.yml`
- `pyproject.toml`
- `CHANGELOG.md`

### Versioning rules
- Version must remain **coherent/consistent** across all versioned files
- Do not update one file and forget the others
- Keep release notes aligned with the actual change set

### Author metadata
- Add yourself as author in `pyproject.toml` if you are contributing to the repository

---

## 11) Pull Request Standard

### Before opening the PR
- Ensure your branch is updated from latest `main`
- Ensure only related changes are included
- Re-run tests and lint checks
- Verify version updates and changelog updates

### PR must include
- Clean summary of the change
- Scope of the change
- Testing details
- Any migration notes
- Any backward compatibility impact

### PR must not include
- Unrelated refactors mixed with feature work
- Temporary debug code
- Commented-out dead code
- Incomplete or untested logic

---

## 12) Required PR Checklist

Use this checklist before marking the PR ready:

- [ ] Branch name follows convention
- [ ] Latest changes pulled from `main`
- [ ] Latest remotes fetched
- [ ] One PR contains only one logical change
- [ ] Code follows Pythonic style
- [ ] DRY principle applied
- [ ] Logic kept simple and readable
- [ ] OOP standards followed
- [ ] Correct design patterns used
- [ ] Factory pattern used for common external/cloud integrations
- [ ] Async/await used where applicable
- [ ] Each class is in a separate `.py` file
- [ ] `snake_case` naming followed
- [ ] All messages come from constants file
- [ ] Proper docstrings added
- [ ] Proper pytest cases added
- [ ] Code coverage > 90%
- [ ] Pylint score > 9.30
- [ ] `run_pylint.bat` executed
- [ ] DB schema/table/column changes discussed if applicable
- [ ] Table/column descriptions added if applicable
- [ ] All DB timestamps stored in UTC
- [ ] Any DB changes done via Alembic
- [ ] `application.yml` updated
- [ ] `pyproject.toml` updated
- [ ] `CHANGELOG.md` updated
- [ ] Version is coherent across files
- [ ] Contributor added as author in `pyproject.toml` if applicable
- [ ] Reviewers added
- [ ] GitHub Copilot added as reviewer (for Python repos)

---

## 13) AI Agent Instructions

If an AI agent is used to generate or modify code in this repository, it must follow all repository standards defined in this file.

### AI agent must do the following
- Read this file before generating code
- Follow Pythonic style
- Keep code simple and maintainable
- Apply DRY
- Use OOP and proper design patterns
- Use async code where appropriate
- Put each class in a separate file
- Use `snake_case`
- Use constants for messages
- Add docstrings
- Add pytest test cases
- Keep coverage above 90%
- Respect pylint requirements
- Update versioned files where required
- Update changelog where required
- Respect DB and Alembic rules strictly

### AI agent must never do the following
- Hardcode messages in logic
- Introduce DB changes without instruction/discussion
- Combine multiple unrelated features in one PR
- Skip tests/docstrings/versioning updates
- Ignore repository conventions defined here

---

## 14) Recommended Commit Hygiene

While not mandatory unless otherwise specified, contributors are encouraged to:
- Write clean and meaningful commit messages
- Avoid noisy WIP commits in final PR history
- Squash/fixup commits if required by team process

Example commit messages:
```text
fix: handle refresh token expiration in oauth service
feat: add async cloud provider factory implementation
refactor: simplify user onboarding validation flow
```

---

## 15) Quick Reference Summary

### Must follow
- Pythonic code
- DRY
- SOLID
- OOP
- Correct design patterns
- Async where applicable
- One class per file
- `snake_case`
- Constants file for all messages
- Docstrings in each file
- Proper pytest tests
- Coverage > 90%
- Pylint > 9.30
- UTC timestamps in DB
- Alembic for DB changes
- Update `application.yml`, `pyproject.toml`, `CHANGELOG.md`
- Add reviewers + GitHub Copilot reviewer
- One PR per logical change

### Must avoid
- Hardcoded messages
- Unapproved DB schema changes
- Multiple features in one PR
- Skipping lint/test/version updates
- Overcomplicated logic

---

## 16) Final Rule

If there is any conflict between convenience and these standards, follow these standards.

Quality, readability, consistency, and reviewability are mandatory for every contribution.