Skip to content

enforce code quality gates (black + flake8 + pytest) with contributor docs and CI blocking #59

Description

@rosspeili

Problem Statement

Problem Statement

Rooms currently relies on informal/local conventions for formatting and linting. Contributors can submit PRs that pass functional tests but still introduce inconsistent style, avoidable lint errors, and noisy diffs. This increases review overhead and causes quality drift over time.

We need a strict, documented, and CI-enforced quality baseline so formatting/lint/test requirements are consistent for all contributors before merge.

Proposed Solution

Introduce a standardized quality toolchain and enforce it in CI:

  1. Adopt baseline tools

    • Formatting: black
    • Linting: flake8
    • Tests: pytest
    • (Optional phase 2) Add pylint only if we agree on rule scope and suppression policy to avoid contributor friction.
  2. Run once on current codebase

    • Apply formatter and fix high-signal lint errors across existing code.
    • Keep this as a dedicated cleanup PR to avoid mixing with feature work.
  3. Enforce in GitHub Actions

    • Update CI workflow to run:
      • python -m black --check .
      • python -m flake8 .
      • PYTHONPATH=. pytest tests/ -v
    • Mark checks as required so non-compliant PRs are blocked from merge.
  4. Document contributor workflow

    • Update CONTRIBUTING.md with required local commands before commit/PR.
    • Update docs/TESTING.md with exact command order and expected behavior.
    • Add short “quality gates” section in README dev/contrib area.
  5. Optional local guardrails

    • Add pre-commit config to run black + flake8 on staged files.
    • Keep optional initially, but recommended in docs.

Alternatives Considered

  • Flake8 only
    • Rejected: doesn’t solve formatting consistency.
  • Pylint as mandatory in v1
    • Rejected for now: too heavy/noisy until baseline is stabilized.
  • Rely on reviewer preference
    • Rejected: inconsistent and not scalable.

Priority

High 🔴

Additional Context

This issue is foundational and should be completed before large incoming feature work (e.g., Skillware integration phases) to keep diffs clean and maintainable.

Suggested rollout:

  • PR A: baseline formatting/lint cleanup (no feature logic changes)
  • PR B: CI enforcement + docs updates
  • PR C (optional): pre-commit onboarding

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or requesthelp wantedExtra attention is neededtestingNew tests or improvements to the testing suite.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions