STYLE: Adding precommit hook and workflow for checking code quality - #302
Jahnvi Thakkar (jahnvi480) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds automated code quality enforcement by introducing pre-commit hooks and GitHub Actions workflows to check Python and C++ code against linting standards.
Key changes:
- GitHub Actions workflow for automatic linting checks on pull requests with configurable thresholds
- Pre-commit hooks for local linting enforcement before commits
- Configuration files defining linting rules, disabled checks, and error thresholds for both Python (pylint) and C++ (cpplint)
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/code-quality-check.yml |
Implements CI workflow to run pylint and cpplint on PRs, report status, and post results as comments |
.pre-commit-config.yml |
Configures pre-commit hooks for pylint and cpplint with specified arguments and file exclusions |
pyproject.toml |
Defines pylint configuration including disabled checks, minimum score threshold of 8.5, and max line length |
cpplint.cfg |
Sets C++ linting rules including line length limit and filtered checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 62.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 79.3%
mssql_python.pybind.connection.connection_pool.cpp: 82.3%
mssql_python.pybind.connection.connection.cpp: 83.1%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.fetch_temporal.hpp: 92.1%🔗 Quick Links
|
Sumit Sarabhai (sumitmsft)
left a comment
Sumit Sarabhai (sumitmsft)
left a comment
There was a problem hiding this comment.
only minor change required. rest all looks good
b942eba
github-actions
Bot
added
pr-size: large
PR Performance Report✅ No regression detectedNo consistent slowdowns detected across all 2 environments. 0 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS Coverage: 2 of 2 environments completed. Advisory result; does not block merging. Performance diagnosticsPhase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed. No affected phases or call-count changes were recorded. All database tasks and timingsUnix / SQL Server 2022
Unix / SQL Server 2025
Build and measurement detailsPR head:
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent. The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes. Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency. Raw samples and logs are attached to the ADO run as |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
devskim found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
b942eba to
cca84e7
Compare
github-actions
Bot
added
pr-size: medium
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 3
Open (4)
paths:filter removed,pull_request.editedwill trigger lint runs for PR… · Newpython -m pipand barepip. In multi-Python environments (including GitHub… · Newpython -m pipinstead ofpipin the… · New
cca84e7 to
7948247
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Canonical setup and validation guides still prescribe the old unpinned Black workflow.
Review effort: Balanced
Findings: 3
Open (5)
python -m pipand barepip. In multi-Python environments (including GitHub…paths:filter removed,pull_request.editedwill trigger lint runs for PR…python -m pipinstead ofpipin the…
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The hook, CI, devcontainer, and documentation changes are consistent and use valid pinned tool releases.
Review effort: Balanced
Findings: 3
Open (5)
python -m pipand barepip. In multi-Python environments (including GitHub…paths:filter removed,pull_request.editedwill trigger lint runs for PR…python -m pipinstead ofpipin the…
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
7948247 to
352b3c1
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The local, devcontainer, and CI formatting paths consistently use the same pinned Black hook without changing informational lint policy.
Review effort: Balanced
Findings: None
Resolved since last review (5)
python -m pipand barepip. In multi-Python environments (including GitHub…paths:filter removed,pull_request.editedwill trigger lint runs for PR…python -m pipinstead ofpipin the…
Work Item / Issue Reference
Summary
Refresh the pre-commit implementation against current
mainso local formattingchecks match the repository's existing blocking CI rule: Black.
mssql_pythonandtests, stopping commits when fixes need review and restaging.files. Run that same hook and pinned formatter version in the existing lint
workflow.
earlier draft's separate Pylint/cpplint blocking workflow rather than introduce
new lint thresholds or change C++ formatting policy.
requirements-lint.txt, install bothhooks automatically in devcontainers, and document setup for new and existing
clones.
in the development environment, and run the shared formatting check before pushes
and PRs. Pulling alone does not execute setup; Copilot acts during an editing task.
required lint status check is not skipped by path filters.
Local hooks cannot prevent PR creation and can be bypassed. Maintainers must
require Linting Summary in branch protection/rulesets to block merging
formatting failures; this PR does not modify repository protection settings.