Improve developer effectiveness: unify quality gates, add Ruff linting, and fast smoke tests#5262
Improve developer effectiveness: unify quality gates, add Ruff linting, and fast smoke tests#5262Manikantasai1724 wants to merge 1 commit intoMDAnalysis:developfrom
Conversation
There was a problem hiding this comment.
Hello there first time contributor! Welcome to the MDAnalysis community! We ask that all contributors abide by our Code of Conduct and that first time contributors introduce themselves on GitHub Discussions so we can get to know you. You can learn more about participating here. Please also add yourself to package/AUTHORS as part of this PR.
There was a problem hiding this comment.
Pull request overview
This PR standardizes and speeds up contributor/CI quality checks by adding pre-commit enforcement, introducing Ruff configuration, and adding a fast “smoke test” gate before running the full CI matrices.
Changes:
- Added pre-commit-driven quality gates (with caching) to Azure Pipelines and GitHub Actions.
- Introduced Ruff configuration and dedicated Ruff CI jobs.
- Added a fast Azure “smoke tests” job and made the full Azure test matrix depend on passing the quick gates.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
azure-pipelines.yml |
Adds pre-commit, Ruff, and smoke-test jobs; gates the main test matrix on these checks. |
.ruff.toml |
Introduces a Ruff configuration file (currently selecting fatal/syntax-level rules). |
.pre-commit-config.yaml |
Adds pre-commit hooks (YAML, whitespace, Ruff, Black) for local + CI enforcement. |
.github/workflows/linters.yaml |
Adds GitHub Actions jobs for pre-commit and Ruff lint checks. |
.github/CONTRIBUTING.md |
Documents running pre-commit locally before opening PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hooks: | ||
| - id: black | ||
| files: ^(package|testsuite)/ | ||
| args: [--line-length=88] |
There was a problem hiding this comment.
The pre-commit Black hook is configured with --line-length=88, but the repository’s canonical Black configuration in package/pyproject.toml and testsuite/pyproject.toml sets line-length = 79. This will cause inconsistent formatting (and likely CI failures against the existing GitHub Actions Black job). Align the pre-commit Black hook with the existing tool.black settings (or remove the explicit --line-length so Black reads from the pyproject config).
| args: [--line-length=88] |
| @@ -0,0 +1,5 @@ | |||
| line-length = 88 | |||
There was a problem hiding this comment.
.ruff.toml sets line-length = 88, but Black is configured for line-length = 79 in both package/pyproject.toml and testsuite/pyproject.toml. Even though the current Ruff rule selection is limited, keeping Ruff’s line-length consistent with Black avoids future drift when additional rules (or formatting) are enabled.
| line-length = 88 | |
| line-length = 79 |
| - job: Ruff_Lint | ||
| displayName: 'Ruff lint checks' | ||
| pool: | ||
| vmImage: 'ubuntu-latest' | ||
| steps: | ||
| - task: UsePythonVersion@0 | ||
| inputs: | ||
| versionSpec: '3.11' | ||
| addToPath: true | ||
| architecture: 'x64' | ||
| - script: python -m pip install --upgrade pip ruff | ||
| displayName: 'Install ruff' | ||
| - script: ruff check package testsuite | ||
| displayName: 'Run ruff checks' | ||
|
|
There was a problem hiding this comment.
Precommit_Lint already runs the Ruff hook from .pre-commit-config.yaml, but this pipeline adds a separate Ruff_Lint job that runs ruff check again. This duplicates work and makes it easier for the two jobs to drift in configuration over time; consider relying on pre-commit as the single source of truth (or remove Ruff from pre-commit and keep it standalone), but avoid running both.
| - job: Ruff_Lint | |
| displayName: 'Ruff lint checks' | |
| pool: | |
| vmImage: 'ubuntu-latest' | |
| steps: | |
| - task: UsePythonVersion@0 | |
| inputs: | |
| versionSpec: '3.11' | |
| addToPath: true | |
| architecture: 'x64' | |
| - script: python -m pip install --upgrade pip ruff | |
| displayName: 'Install ruff' | |
| - script: ruff check package testsuite | |
| displayName: 'Run ruff checks' |
| cython | ||
| h5py>=2.10 | ||
| matplotlib | ||
| numpy | ||
| packaging | ||
| pytest | ||
| tqdm | ||
| threadpoolctl | ||
| filelock |
There was a problem hiding this comment.
The smoke job installs a large set of unpinned runtime/test dependencies (e.g., numpy/matplotlib/pytest) directly from PyPI. Because this job is now a required quality gate for the full matrix, upstream releases can introduce flaky CI failures unrelated to the PR. Consider pinning these to a known-good constraints file (or to the same minimum/compatible versions used elsewhere in CI) to keep the smoke gate stable.
| cython | |
| h5py>=2.10 | |
| matplotlib | |
| numpy | |
| packaging | |
| pytest | |
| tqdm | |
| threadpoolctl | |
| filelock | |
| cython==3.0.8 | |
| h5py==3.10.0 | |
| matplotlib==3.8.2 | |
| numpy==1.26.4 | |
| packaging==23.2 | |
| pytest==7.4.4 | |
| tqdm==4.66.1 | |
| threadpoolctl==3.2.0 | |
| filelock==3.13.1 |
| precommit: | ||
| if: "github.repository == 'MDAnalysis/mdanalysis'" | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| env: | ||
| PRE_COMMIT_HOME: ~/.cache/pre-commit | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: cache pre-commit | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.cache/pre-commit | ||
| key: pre-commit-${{ runner.os }}-${{ hashFiles('.pre-commit-config.yaml') }} | ||
|
|
||
| - name: install pre-commit | ||
| run: | | ||
| python -m pip install --upgrade pip pre-commit | ||
|
|
||
| - name: run pre-commit | ||
| run: | | ||
| pre-commit run --all-files | ||
|
|
||
| ruff_check: | ||
| if: "github.repository == 'MDAnalysis/mdanalysis'" | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: install ruff | ||
| run: | | ||
| python -m pip install --upgrade pip ruff | ||
|
|
||
| - name: run ruff | ||
| run: | | ||
| ruff check package testsuite | ||
|
|
There was a problem hiding this comment.
The new precommit job runs the Ruff and Black hooks (per .pre-commit-config.yaml), but this workflow also runs Ruff again (ruff_check) and still has a dedicated black job. This triples the same checks and increases CI time; consider either (1) using pre-commit as the single quality gate and dropping the redundant Ruff/Black jobs, or (2) configuring pre-commit here to run only the non-duplicated hooks.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5262 +/- ##
===========================================
- Coverage 93.83% 93.82% -0.01%
===========================================
Files 180 180
Lines 22473 22473
Branches 3189 3189
===========================================
- Hits 21088 21086 -2
- Misses 923 924 +1
- Partials 462 463 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary:
This PR improves development workflow reliability and CI efficiency by standardizing quality checks across local development and both CI providers.
What changed:
Introduced pre-commit-based local/CI quality enforcement.
Added cache configuration to speed up repeated pre-commit runs.
Added Ruff lint configuration and CI jobs for static checks.
Added a smoke-test job to fail fast on critical issues.
Made full matrix tests depend on passing quick quality gates.
Updated contributor instructions for local quality-check workflow.
Why:
Reduce avoidable CI failures.
Provide faster feedback to contributors.
Lower CI resource usage by preventing expensive runs when basic checks fail.
Keep local and CI behavior consistent.
Validation performed:
Verified updated workflow/config files are syntactically valid.
Confirmed no editor-reported errors in modified CI and config files.
Impact/Risk:
CI may initially fail on newly surfaced lint issues, which is expected and improves code health.
No runtime/library behavior changes; impact is limited to workflow/tooling.
📚 Documentation preview 📚: https://mdanalysis--5262.org.readthedocs.build/en/5262/