asf-tooling commented on issue #502:
URL:
https://github.com/apache/tooling-trusted-releases/issues/502#issuecomment-4407580032
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@751c2146`
**Type:** `new_feature` • **Classification:** `actionable` •
**Confidence:** `medium`
**Application domain(s):** `infrastructure`
### Summary
This issue requests integrating the existing
`fix_order.sh`/`interface_order.py` function-ordering check into the pre-commit
linting workflow. The tooling already exists (`scripts/fix_order.py`,
`scripts/fix_order.sh`, `scripts/interface_order.py`), and @sbp noted in the
Feb 2026 discussion that commit ece6e9d4 made significant advances—all but
three `.py` files in `atr/` are now compatible. What remains is the
pre-commit/CI integration step. The `.pre-commit-config.yaml` file was not
provided in the inventory, so the exact current hook configuration is unknown,
but `pre-commit>=2.20.0` is a dev dependency.
### Where this lives in the code today
#### `scripts/interface_order.py` — `main` (lines 67-86)
_needs modification_
Currently accepts exactly one file; pre-commit passes multiple files as
arguments, so this needs to support multiple file paths.
```python
def main() -> None:
quiet = sys.argv[2:3] == ["--quiet"]
argc = len(sys.argv)
match (argc, quiet):
case (2, False):
...
case (3, True):
...
case _:
print("Usage: python interface_order.py <filename> [ --quiet ]",
file=sys.stderr)
sys.exit(ExitCode.USAGE_ERROR)
file_path = pathlib.Path(sys.argv[1])
all_ok = check_order(file_path, quiet)
if all_ok:
if not quiet:
print(f"ok {file_path}")
sys.exit(ExitCode.SUCCESS)
else:
sys.exit(ExitCode.FAILURE)
```
#### `scripts/interface_order.py` — `check_order` (lines 39-64)
_currently does this_
The per-file check logic that would be called in a loop for multi-file
support.
```python
def check_order(file_path: pathlib.Path, quiet: bool) -> bool:
content = _read_file_content(file_path)
if content is None:
sys.exit(ExitCode.FAILURE)
tree = _parse_python_code(content, str(file_path))
if tree is None:
sys.exit(ExitCode.FAILURE)
class_names = _extract_top_level_class_names(tree)
function_names = _extract_top_level_function_names(tree)
all_ok = True
if not _verify_names_are_sorted(function_names, str(file_path),
"function"):
all_ok = False
for class_name in class_names:
if class_name.startswith("_"):
print(f"!! {file_path} - class '{class_name}' is private",
file=sys.stderr)
all_ok = False
if (not quiet) or (str(file_path) not in _CLASS_EXEMPTIONS):
if not _verify_names_are_sorted(class_names, str(file_path),
"class"):
all_ok = False
return all_ok
```
### Where new code would go
- `.pre-commit-config.yaml` — existing file (not provided in inventory) —
add a new local hook entry
The pre-commit hook entry for function ordering check would be added here.
### Proposed approach
The integration has two parts: (1) modify `scripts/interface_order.py` to
accept multiple file arguments so it can work efficiently with pre-commit
(which passes all staged files at once), and (2) add a local pre-commit hook
entry that runs `interface_order.py --quiet` on staged `.py` files. Using
`--quiet` mode in pre-commit suppresses the 'ok' messages and only outputs
errors.
The `interface_order.py` script's `main()` function needs to be updated to
iterate over multiple file arguments. The hook should use `language: system`
since it invokes `uv run`, and `types: [python]` to filter to `.py` files. This
keeps the check fast (AST parsing is quick) while catching ordering violations
before they hit CI.
### Suggested patches
#### `scripts/interface_order.py`
Support multiple file arguments for pre-commit compatibility, which passes
all matching staged files as positional args.
````diff
--- a/scripts/interface_order.py
+++ b/scripts/interface_order.py
@@ -64,24 +64,26 @@
return all_ok
def main() -> None:
- quiet = sys.argv[2:3] == ["--quiet"]
- argc = len(sys.argv)
- match (argc, quiet):
- case (2, False):
- ...
- case (3, True):
- ...
- case _:
- print("Usage: python interface_order.py <filename> [ --quiet
]", file=sys.stderr)
- sys.exit(ExitCode.USAGE_ERROR)
-
- file_path = pathlib.Path(sys.argv[1])
- all_ok = check_order(file_path, quiet)
- if all_ok:
- if not quiet:
- print(f"ok {file_path}")
- sys.exit(ExitCode.SUCCESS)
- else:
+ args = sys.argv[1:]
+ quiet = False
+ if "--quiet" in args:
+ quiet = True
+ args.remove("--quiet")
+
+ if not args:
+ print("Usage: python interface_order.py [--quiet] <filename>
[<filename> ...]", file=sys.stderr)
+ sys.exit(ExitCode.USAGE_ERROR)
+
+ all_ok = True
+ for filename in args:
+ file_path = pathlib.Path(filename)
+ if not check_order(file_path, quiet):
+ all_ok = False
+ elif not quiet:
+ print(f"ok {file_path}")
+
+ if all_ok:
+ sys.exit(ExitCode.SUCCESS)
+ else:
sys.exit(ExitCode.FAILURE)
````
#### `.pre-commit-config.yaml`
Add local hook for function ordering check. Since this file was not in the
inventory, this is an illustrative snippet to be merged into the existing
config.
````diff
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ ... (add to existing hooks list) ...
+ - repo: local
+ hooks:
+ - id: check-function-order
+ name: Check function ordering
+ entry: uv run --frozen python3 scripts/interface_order.py --quiet
+ language: system
+ types: [python]
+ pass_filenames: true
````
### Open questions
- The .pre-commit-config.yaml was not included in the provided file
inventory — the exact existing hook configuration is unknown, so the proposed
hook entry needs to be placed appropriately within whatever structure already
exists.
- Are there still files that cannot pass the ordering check (the 'three
files' @sbp mentioned)? If so, an exclude pattern may be needed in the hook
definition.
- Should the hook also offer auto-fix capability (e.g., a separate hook ID
that runs fix_order.sh), or is the check-only approach sufficient for the
pre-commit workflow?
- The issue references #361 as related — unclear if that issue covers other
aspects of this integration that might affect the approach.
### Files examined
- `scripts/fix_order.py`
- `scripts/fix_order.sh`
- `scripts/fix_orders.sh`
- `scripts/interface_order.py`
- `pyproject.toml`
- `.asf.yaml`
- `.github/PULL_REQUEST_TEMPLATE.md`
- `.github/dependabot.yml`
---
*Draft from a triage agent. A human reviewer should validate before merging
any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
