From 352b3c14a0296eeac005da3f1edf46adec586d09 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Thu, 1 Oct 2026 09:59:42 +0530 Subject: [PATCH] CHORE: Add CI-aligned formatting hooks and Copilot setup guidance Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .devcontainer/post-create.sh | 10 ++- .github/copilot-instructions.md | 6 +- .../local-formatting.instructions.md | 67 +++++++++++++++++++ .github/prompts/setup-dev-env.prompt.md | 24 +++++-- .github/workflows/lint-check.yml | 39 +++++------ .pre-commit-config.yaml | 20 ++++++ CONTRIBUTING.md | 60 +++++++++++++++++ requirements-lint.txt | 2 + 8 files changed, 199 insertions(+), 29 deletions(-) create mode 100644 .github/instructions/local-formatting.instructions.md create mode 100644 .pre-commit-config.yaml create mode 100644 requirements-lint.txt diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index 45e42d111..16341d762 100755 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -7,8 +7,12 @@ echo "🚀 Setting up MSSQL Python Driver development environment..." # Install Python packages from requirements.txt echo "📦 Installing Python packages..." -pip install --upgrade pip setuptools wheel -pip install -r requirements.txt +python -m pip install --upgrade pip setuptools wheel +python -m pip install -r requirements.txt +python -m pip install -r requirements-lint.txt + +echo "Installing commit and push formatting hooks..." +python -m pre_commit install --install-hooks # Create symlink for 'python' command (build.sh expects it) echo "🔗 Creating python symlink..." @@ -30,7 +34,7 @@ grep -qxF 'source ~/.shell_aliases' ~/.zshrc 2>/dev/null || echo 'source ~/.shel echo "" echo "🔍 Verifying environment..." python --version -pip --version +python -m pip --version cmake --version if command -v sqlcmd &> /dev/null; then echo "✅ sqlcmd available" diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6ade59b02..5b6322070 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -22,6 +22,10 @@ Paths you will touch: ## Development workflow +For local editing, commit, or push tasks, follow +[local formatting setup](instructions/local-formatting.instructions.md) to install +the hooks when needed and use the same pinned formatting check as CI. + Validated, step-by-step guides live in `.github/prompts/` — use them instead of rediscovering commands: - `setup-dev-env.prompt.md` — venv, dependencies, ODBC headers, `DB_CONNECTION_STRING`, SQL Server. @@ -38,7 +42,7 @@ Core facts: ## Validation gate (run before you finish — this mirrors CI) ```bash -black --check --line-length=100 mssql_python/ tests/ # BLOCKING in CI +python -m pre_commit run black-check --all-files --hook-stage pre-push # BLOCKING in CI python -m pytest -v # 'stress' marker excluded by default ``` diff --git a/.github/instructions/local-formatting.instructions.md b/.github/instructions/local-formatting.instructions.md new file mode 100644 index 000000000..1b5f63107 --- /dev/null +++ b/.github/instructions/local-formatting.instructions.md @@ -0,0 +1,67 @@ +--- +applyTo: "**" +--- + +# Local formatting setup + +When starting a local editing, commit, or push task, ensure this clone has the +repository's formatting hooks installed. Perform the setup when needed rather +than only suggesting commands. Do not install tools or change hooks for read-only +questions, code reviews, or CI jobs. Respect the user's tool permissions. + +These instructions take effect when Copilot works on the checkout. Pulling +`main` does not execute them, and a developer without an active Copilot session +must follow the setup in `CONTRIBUTING.md`. + +## One-time setup per clone + +1. Use the project's Python 3.10+ virtual environment, or the devcontainer's + configured Python. If no development environment exists, follow + `.github/prompts/setup-dev-env.prompt.md` for virtual environment setup first. + Do not install into an unrelated or global Python environment. +2. Check `python -m pre_commit --version` against `requirements-lint.txt`. + If the module is missing or its version differs, install it: + + ```console + python -m pip install -r requirements-lint.txt + ``` + +3. Inspect `git config --get core.hooksPath` and resolve the default hook directory + with `git rev-parse --git-path hooks`; do not assume `.git` is a directory. + If a custom hooks path is configured, stop and ask how to integrate with it; + do not unset it or change global Git settings. Check that both `pre-commit` and + `pre-push` are pre-commit-managed hooks for `.pre-commit-config.yaml` and use a + valid development interpreter. If missing or stale, run: + + ```console + python -m pre_commit install --install-hooks + ``` + + Preserve existing hooks; do not use `--overwrite`. Verify both hooks were + installed. Skip reinstallation when they are already correct, including in + devcontainers. Reinstall if the development environment was recreated. +4. Verify the configuration with `python -m pre_commit validate-config`. + Report installation, network, or environment errors explicitly. Do not claim + setup succeeded or bypass a failure. + +## Before commits, pushes, and PRs + +- Let the commit hook format staged Python files. If it changes files, inspect + the diff and stage only the intended fixes before retrying. Preserve unrelated + and partially staged edits; never use `git add .` to accept formatter changes. +- Before pushing or opening a PR, run the same full-directory check as CI: + + ```console + python -m pre_commit run black-check --all-files --hook-stage pre-push + ``` + +- To fix reported formatting errors, use + `python -m pre_commit run black --all-files`. It returns nonzero when it changes + files. Review the diff, keep unrelated edits out of the commit, and rerun the + check. Do not commit or push unless the user has authorized those actions. +- Use the pinned hook environment, not a separately installed Black version. + These formatting checks need neither a native extension build nor SQL Server. + Keep other linters informational, matching the existing CI policy. +- Never use `--no-verify`, `SKIP`, or configuration changes to bypass the checks. + Local hooks cannot enforce PR creation or merging: requiring **Linting Summary** + in branch protection is a separate administrator action, not part of setup. diff --git a/.github/prompts/setup-dev-env.prompt.md b/.github/prompts/setup-dev-env.prompt.md index 950034067..ce126008b 100644 --- a/.github/prompts/setup-dev-env.prompt.md +++ b/.github/prompts/setup-dev-env.prompt.md @@ -124,10 +124,19 @@ pip install pybind11 # Test dependencies pip install pytest pytest-cov -# Linting/formatting (optional) -pip install black flake8 autopep8 +# Install the shared local/CI formatting tools +python -m pip install -r requirements-lint.txt +python -m pre_commit install --install-hooks ``` +Both commit and push hooks are required for contributors. Commit hooks format +staged Python files; push hooks run the same full-directory Black check as CI. +If formatting changes a file, review and stage it before retrying the commit. +Run `python -m pre_commit run black-check --all-files --hook-stage pre-push` +before opening a PR. See `CONTRIBUTING.md` for setup and enforcement details. +Copilot should follow `.github/instructions/local-formatting.instructions.md` +to check for existing hooks and custom hooks paths before installing. + ### 3.4 Install Package in Development Mode ```bash @@ -737,10 +746,12 @@ Set-ExecutionPolicy -ExecutionPolicy RemoteSigned -Scope CurrentUser # Complete setup from scratch python3 -m venv myvenv && \ source myvenv/bin/activate && \ -pip install --upgrade pip && \ -pip install -r requirements.txt && \ -pip install pybind11 pytest pytest-cov && \ -pip install -e . && \ +python -m pip install --upgrade pip && \ +python -m pip install -r requirements.txt && \ +python -m pip install -r requirements-lint.txt && \ +python -m pre_commit install --install-hooks && \ +python -m pip install pybind11 pytest pytest-cov && \ +python -m pip install -e . && \ echo "✅ Setup complete!" ``` @@ -752,6 +763,7 @@ echo "✅ Setup complete!" | `pytest` | Testing | Running tests | | `pytest-cov` | Coverage | Coverage reports | | `azure-identity` | Azure auth | Runtime (in requirements.txt) | +| `pre-commit` | Pinned Black hooks | Commits and pushes (in requirements-lint.txt) | --- diff --git a/.github/workflows/lint-check.yml b/.github/workflows/lint-check.yml index 224fea494..234d3d004 100644 --- a/.github/workflows/lint-check.yml +++ b/.github/workflows/lint-check.yml @@ -2,18 +2,7 @@ name: Linting Check on: pull_request: - types: [opened, edited, reopened, synchronize] - - paths: - - '**.py' - - '**.cpp' - - '**.c' - - '**.h' - - '**.hpp' - - '.github/workflows/lint-check.yml' - - 'pyproject.toml' - - '.flake8' - - '.clang-format' + types: [opened, reopened, synchronize] push: branches: - main @@ -33,22 +22,34 @@ jobs: persist-credentials: false - name: Set up Python + id: python uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 with: python-version: '3.13' cache: 'pip' + cache-dependency-path: | + requirements.txt + requirements-lint.txt + .pre-commit-config.yaml - name: Install dependencies run: | python -m pip install --upgrade pip - pip install black flake8 pylint autopep8 - if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + python -m pip install -r requirements-lint.txt + python -m pip install flake8 pylint autopep8 + if [ -f requirements.txt ]; then python -m pip install -r requirements.txt; fi + + - name: Cache pre-commit environments + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.cache/pre-commit + key: ${{ runner.os }}-${{ runner.arch }}-pre-commit-${{ steps.python.outputs.python-version }}-${{ hashFiles('.pre-commit-config.yaml', 'requirements-lint.txt') }} - name: Check Python formatting with Black run: | echo "::group::Black Formatting Check" - black --check --line-length=100 --diff mssql_python/ tests/ || { - echo "::error::Black formatting issues found. Run 'black --line-length=100 mssql_python/ tests/' locally to fix." + python -m pre_commit run black-check --all-files --hook-stage pre-push || { + echo "::error::Black formatting issues found. Run 'python -m pre_commit run black --all-files' locally, stage the fixes, and retry." exit 1 } echo "::endgroup::" @@ -73,7 +74,7 @@ jobs: - name: Check Type Hints (mypy) run: | echo "::group::Type Checking" - pip install mypy + python -m pip install mypy mypy mssql_python/ --ignore-missing-imports --no-strict-optional --check-untyped-defs || { echo "::warning::Type checking found potential issues. Review the output above." } @@ -104,7 +105,7 @@ jobs: - name: Install cpplint run: | python -m pip install --upgrade pip - pip install cpplint + python -m pip install cpplint - name: Check C++ formatting with clang-format run: | @@ -173,7 +174,7 @@ jobs: echo "" >> $GITHUB_STEP_SUMMARY echo "### How to Fix" >> $GITHUB_STEP_SUMMARY echo "1. Save all files in VS Code (Ctrl+S) - auto-formatting will fix most issues" >> $GITHUB_STEP_SUMMARY - echo "2. Or run manually: \`black --line-length=100 mssql_python/ tests/\`" >> $GITHUB_STEP_SUMMARY + echo "2. Or run manually: \`python -m pre_commit run black --all-files\`, then stage the fixes" >> $GITHUB_STEP_SUMMARY echo "3. For C++: \`clang-format -i mssql_python/pybind/*.cpp\`" >> $GITHUB_STEP_SUMMARY - name: Fail if Python formatting failed diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..117127be3 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,20 @@ +minimum_pre_commit_version: '4.0.0' +default_install_hook_types: [pre-commit, pre-push] + +repos: + - repo: https://github.com/psf/black-pre-commit-mirror + rev: 26.5.1 + hooks: + - id: black + name: Black (format staged Python files) + files: ^(mssql_python|tests)/.*\.pyi?$ + types_or: [python, pyi] + args: [--config=pyproject.toml] + stages: [pre-commit, manual] + - id: black + alias: black-check + name: Black (full CI formatting check) + args: [--config=pyproject.toml, --check, --diff, mssql_python, tests] + pass_filenames: false + always_run: true + stages: [pre-push] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fdb5ba8ec..83ff7dd67 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,6 +15,66 @@ or contact [opencode@microsoft.com](mailto:opencode@microsoft.com) with any addi ## Before Contributing +### Install Local Formatting Hooks + +From the repository root, with your Python 3.10+ development virtual environment +activated, run this once per clone (including existing clones): + +```console +python -m pip install -r requirements-lint.txt +python -m pre_commit install --install-hooks +``` + +This installs both **pre-commit** and **pre-push** hooks on Windows, macOS, and +Linux. Devcontainers install them automatically. Git does not install hooks when +cloning, and installing Python dependencies alone does not activate them. Rerun +the commands if you recreate your virtual environment. + +Copilot setup guidance lives in +[local-formatting.instructions.md](.github/instructions/local-formatting.instructions.md). +It directs Copilot to perform missing setup when starting a local editing task, +subject to tool permissions. Pulling `main` alone does not run setup. + +- **On commit:** Black formats staged `.py` and `.pyi` files under `mssql_python` + and `tests`. If it changes a file, the commit stops; review and stage the fixes, + then commit again. Unstaged changes are temporarily saved and restored by + pre-commit. +- **On push:** Black checks both directories in full, without modifying files, + and blocks the push on formatting errors. This also catches formatting errors + outside the files changed in your latest commit. +- **In CI:** the same full-directory hook, pinned Black version, and + `pyproject.toml` settings are used. Flake8, Pylint, mypy, clang-format, and + cpplint remain informational, matching the existing CI policy. + +Run the CI formatting check before opening a PR: + +```console +python -m pre_commit run black-check --all-files --hook-stage pre-push +``` + +To fix formatting, run the pinned formatter, review the changes, and stage the +affected files before retrying your commit or push: + +```console +python -m pre_commit run black --all-files +``` + +The formatter returns a nonzero status when it changes files; rerun after +reviewing the fixes. Hook environments need network access on first installation +and when the pinned version changes. The checks do not need a native build or a +SQL Server. + +**Repository enforcement:** local hooks can be bypassed and cannot prevent +someone from opening a GitHub PR. Maintainers must require the **Linting Summary** +status check in the `main` branch ruleset/branch protection, with bypasses +restricted, to prevent merging a PR with failing formatting. The lint workflow +runs for every PR, including documentation-only changes, so a required check is +not left pending by path filters. Hook installation is a contributor setup step, +not a repository-wide setting. + +When upgrading Black, update its revision in `.pre-commit-config.yaml` and run +both commands above; local hooks and CI then use the new version together. + ### For External Contributors If you are an external contributor (not a Microsoft organization member), please follow these steps: diff --git a/requirements-lint.txt b/requirements-lint.txt new file mode 100644 index 000000000..f2ea9cb29 --- /dev/null +++ b/requirements-lint.txt @@ -0,0 +1,2 @@ +# Black is pinned in .pre-commit-config.yaml and installed in an isolated hook environment. +pre-commit==4.5.1