Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

RfC: Enforce consistent formatting on CI #1957

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/fpga-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ env:

jobs:
test-fpga:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'no-ci') }}
if: False # ${{ !contains(github.event.pull_request.labels.*.name, 'no-ci') }}
runs-on: [self-hosted, linux, intel-fpga, xilinx-fpga]
steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/general-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ on:

jobs:
test:
if: "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
if: False # "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
runs-on: ubuntu-latest
strategy:
matrix:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/gpu-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ env:

jobs:
test-gpu:
if: "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
if: False # "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
runs-on: [self-hosted, gpu]
steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/heterogeneous-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ env:

jobs:
test-heterogeneous:
if: "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
if: False # "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
runs-on: [self-hosted, linux]
steps:
- uses: actions/checkout@v4
Expand Down
29 changes: 29 additions & 0 deletions .github/workflows/linting.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Code Quality

on:
push:
branches: [ main, ci-fix ]
pull_request:
branches: [ main, ci-fix ]

jobs:
linting:
if: "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
name: pre-commit
runs-on: ubuntu-latest

steps:
- name: Check repository
uses: actions/checkout@v4

- name: Setup Python 3.13
uses: actions/setup-python@v5
with:
python-version: '3.13'
cache: 'pip'

- name: Install linting tools
run: pip install pre-commit yapf

- name: Run linting tools
run: pre-commit run --all-files
2 changes: 1 addition & 1 deletion .github/workflows/pyFV3-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ defaults:

jobs:
build_and_validate_pyFV3:
if: "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
if: False # "!contains(github.event.pull_request.labels.*.name, 'no-ci')"
runs-on: ubuntu-latest
strategy:
matrix:
Expand Down
20 changes: 20 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
default_language_version:
python: python3

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: check-merge-conflict
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/google/yapf
rev: v0.43.0
hooks:
- id: yapf
name: yapf
language: python
entry: yapf
args: [-i]
types: [python]
23 changes: 21 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,28 @@ def example_function(param_a: str, *args: Optional[SDFG]) -> bool:
...
```

For automatic styling, we rely on [pre-commit](https://pre-commit.com/) and use the [yapf](https://github.com/google/yapf) file formatter.
**Please run `pre-commit` before making your pull request ready for review.**

For automatic styling, we use the [yapf](https://github.com/google/yapf) file formatter.
**Please run `yapf` before making your pull request ready for review.**
```bash
pre-commit run --all-files
```

Formatting will be evaluated as part of CI and you won't be able to merge unless formatting issues are resolved. To get `pre-commit` and `yapf`, be sure to install the `linting` extra, e.g. for contributors we recommend

```bash
pip install -e ".[testing,linting]"
```

to get testing and linting extras in an editable install.

One way to always ensure consistency with our coding standards is to install pre-commit hooks, which will check formatting issues of changed files before every commit. If you wish to do so, run

```bash
pre-commit install
```

to install the `git` hooks for this repository.

## Tests

Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@
] + cmake_requires,
extras_require={
'testing': ['coverage', 'pytest-cov', 'scipy', 'absl-py', 'opt_einsum', 'pymlir', 'click', 'ipykernel', 'nbconvert'],
'docs': ['jinja2<3.2.0', 'sphinx-autodoc-typehints', 'sphinx-rtd-theme>=0.5.1']
'docs': ['jinja2<3.2.0', 'sphinx-autodoc-typehints', 'sphinx-rtd-theme>=0.5.1'],
'linting': ['pre-commit', 'yapf'],
},
entry_points={
'console_scripts': [
Expand Down
Loading