Skip to content

ci: make the ruff lint and format steps able to fail - #598

Merged
Corina (corinagum) merged 2 commits into
mainfrom
cg/ci-format-check
Sep 16, 2026
Merged

Corina (corinagum) merged 2 commits into
mainfrom
cg/ci-format-check

Conversation

@corinagum

@corinagum Corina (corinagum) commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

The Format step ran ruff format, which rewrites files in place and exits 0, so it passed unconditionally. Proof it was a no-op: install_update/__init__.py was unformatted on main while main was green. The Lint step had the same defect via fix = true in pyproject, which makes a bare ruff check auto-fix and exit 0.

Changes

  • ci.yml: both ruff steps now run uv run --frozen from the venv the sync step already builds, instead of astral-sh/ruff-action. The action resolved ruff from the >=0.11.13 floor and installed 0.16.8, ignoring the 0.14.1 pin in uv.lock. That skew is real: 0.16.8 flagged 8 correctly-formatted files. uv.lock is now the single source of truth, matching how the PyRight and test steps already resolve tools.
  • install_update/__init__.py: one stray blank line removed. The only file the pinned ruff flags, and the workflow change goes red without it.

Verification

Against a deliberately malformed file the old commands exited 0 and rewrote it; the new ones exit 1 and leave it untouched. CI now reports 513 files, matching local exactly.

Removes a stray second blank line between the module docstring and the imports, which is the only file in the workspace that `ruff format --check` reports as needing reformatting.

This lands ahead of the CI change so each commit is independently green.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 16, 2026 21:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The CI changes and required formatting correction are complete with no unresolved review issues.

Pull request overview

Updates CI so Ruff linting and formatting fail on violations instead of silently fixing files.

Changes:

  • Use ruff check --no-fix and ruff format --check.
  • Apply the required formatting correction.
File summaries
File Summary
packages/api/src/microsoft_teams/api/activities/install_update/__init__.py Removes the extra blank line flagged by Ruff.
.github/workflows/ci.yml Enforces non-mutating Ruff validation.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The Format step ran `ruff format`, which rewrites files in place and exits 0. On an ephemeral runner nothing consumes the rewrite, so the step passed unconditionally. Proof that it was a no-op: `packages/api/src/microsoft_teams/api/activities/install_update/__init__.py` was unformatted on main while main was green.

The Lint step had the same defect for a different reason: `fix = true` in the root pyproject `[tool.ruff]` table makes a bare `ruff check` auto-fix and exit 0, so any auto-fixable violation passed silently.

Both steps now run ruff from the venv that the preceding sync step already built, rather than through astral-sh/ruff-action. The action resolved its own ruff from the `ruff>=0.11.13` floor in pyproject and installed the newest matching release, ignoring the 0.14.1 pin in uv.lock that contributors and `poe check` use. That skew is not theoretical: the action installed 0.16.8, which formats Python blocks inside Markdown and parenthesizes multi-line lambda bodies, so it reported eight files that are correctly formatted under the pinned version. Running through `uv run --frozen` makes uv.lock the single source of truth, so CI and local can no longer disagree, and matches how the PyRight and test steps in this workflow already resolve their tools.

Verified both directions against a deliberately malformed file: the old commands exited 0 and rewrote it, the new ones exit 1 and leave it untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@corinagum
Corina (corinagum) merged commit a669054 into main Sep 16, 2026
8 checks passed
@corinagum
Corina (corinagum) deleted the cg/ci-format-check branch September 16, 2026 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants