Skip to content

ci(hooks): use managed Python environments - #305

Merged
SigureMo merged 1 commit into
redai-studio:mainfrom
ooooo-create:ci/pre-commit-python-hooks
Sep 8, 2026
Merged

ci(hooks): use managed Python environments#305
SigureMo merged 1 commit into
redai-studio:mainfrom
ooooo-create:ci/pre-commit-python-hooks

Conversation

@ooooo-create

@ooooo-create ooooo-create commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What

Run the check-conflict-markers and format-ascii-boxes local hooks in pre-commit-managed Python 3 environments.

Why

Both hooks invoke python. With language: system, they depend on a python executable on the caller's PATH, which can fail on systems that only expose python3.

How

Set language: python and language_version: python3 for both hooks so pre-commit provisions their Python environments.

Testing

pre-commit run --all-files --show-diff-on-failure passed with the existing pre-commit environment's bin directory on PATH.

pytest tests/ was not run because this change only updates hook configuration. Multi-node GPU integration tests were skipped because they require multi-node GPU hardware and this change does not affect training. New tests and documentation updates are not applicable.

  • pre-commit run --all-files passes
  • Tests pass (pytest tests/)
  • New tests added (if applicable)
  • Documentation updated (if applicable)

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • CI/CD or build changes

Screenshots / Logs

# 🔧 CI/CD

- Run check-conflict-markers and format-ascii-boxes in pre-commit-managed Python environments.
- Explicitly select Python 3 for both local hooks.
Copilot AI lite review requested due to automatic review settings September 8, 2026 14:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The config still runs the gitleaks wrapper via python ..., so pre-commit run --all-files can still fail on systems without a python executable even after this update.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the repository’s local pre-commit hooks so they run in pre-commit-managed Python 3 environments instead of relying on a python executable being present on the user’s PATH.

Changes:

  • Switch check-conflict-markers local hook from language: system to language: python.
  • Switch format-ascii-boxes local hook from language: system to language: python.
  • Pin both hooks to language_version: python3 so pre-commit provisions a Python 3 interpreter for them.
File summaries
File Description
.pre-commit-config.yaml Moves two local hooks to pre-commit-managed Python environments (Python 3) to avoid relying on python on PATH.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment thread .pre-commit-config.yaml
Comment on lines 67 to +71
- id: format-ascii-boxes
name: format-ascii-boxes
entry: python .pre-commit-hooks/format_ascii_boxes.py
language: system
language: python
language_version: python3

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

已审查完整 diff 及两个 hook 的实现,未发现需要阻塞合入的问题。改动范围与 PR 描述一致,Pre-commit Checks 已通过;单元测试矩阵仍在运行。本地未重跑检查,多节点 GPU 集成测试因缺少相应硬件未执行。

Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.

@SigureMo SigureMo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTMeow 🐾

@SigureMo
SigureMo merged commit 6edb628 into redai-studio:main Sep 8, 2026
4 checks passed
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.

4 participants