Skip to content

Commit ba722a1

Browse files
krowvinkrowvin
authored
Feat: Centralized maintainer ownership for CLI help and docs (#180)
**Fishing for thoughts on this: Do it, part of it, none of it, tweak it?** Long-term I think we want to consider one or more DLL or mailing lists for this to help with turnover. Could even use CDA to query for a group of emails over a specific command. Adds a centralized maintainer/ownership workflow for cwms-cli uses it to surface maintainers in both the CLI help output and the docs. This adds a user-friendly contact path for issue reporting. It is optional where ownership is not yet defined, and is intended to encourage users to report issues. *Issues* should always be created for feature requests, bugs, etc. Having authors on sub commands helps provide this contact path where making an issue might otherwise go unreported. Having some email to report something would enable maintainers to create followup issues. (Issues do report a link to the github issues page for cwms-cli!) ## What this does This introduces a single ownership source of truth in `cwms-cli/maintainers.toml`. From that file, the repo now generates and maintains: - cwms-cli/.github/CODEOWNERS for GitHub review/accountability - Poetry authors in cwms-cli/pyproject.toml - runtime ownership metadata in cwms-cli/cwmscli/_generated/ownership_data.py - docs include fragments under cwms-cli/docs/_generated/maintainers That generated data is then used in two user-facing places: - CLI help output shows `Maintainers:` on help pages <img width="588" height="328" alt="image" src="https://github.com/user-attachments/assets/8ba3bf0a-33d7-49b8-b233-5bd39834fa9e" /> _Each command has author(s) associated_ - docs pages now show a maintainer note sourced from the same ownership file ## Implementation details The sync flow is handled by cwms-cli/scripts/sync_ownership.py Current command ownership mapping: - Charles: csv2cwms, blob, update, shared cwms-cli, shared load - Eric: usgs, shefcritimport, shared cwms-cli, shared load CLI help rendering was extended in `cwms-cli/cwmscli/utils/click_help.py` to show maintainers centrally, without hand-editing each command. Ownership lookup is handled by `cwms-cli/cwmscli/ownership.py`, and nested commands inherit from the nearest owned parent command. Docs were updated to include generated maintainer blocks on the current command pages: - docs/cli.rst - docs/cli/blob.rst - docs/cli/csv2cwms.rst - docs/cli/update.rst - docs/cli/load_location_ids_all.rst ## Contributing / maintenance cwms-cli/CONTRIBUTING.md was updated to document the new workflow. Contributors now have explicit guidance to: - treat maintainers.toml file to maintain contributors in - run python scripts/sync_ownership.py after ownership edits - use python scripts/sync_ownership.py --check to verify generated files There is also enforcement in both local hooks and CI: - pre-commit now runs a generated ownership files check *(Debatable if we need this?)* - GitHub Actions verifies generated ownership files are in sync ~(This is probably enough to make sure it is not forgotten)~ ## Benefits This gives `cwms-cli` a clear ownership model with one place to maintain it. Benefits: - maintainers are visible to users in CLI help and docs - ownership and package author metadata stay aligned - GitHub review routing now has an explicit CODEOWNERS base - command ownership is easier to update during turnover - changes to ownership are reviewable as normal repo diffs - the same ownership data can be extended later without adding more manual duplication ## Alternatives considered ### Git history / recent commits This was considered but not used as the primary maintainer source. Reason: - recent commits do not reliably indicate who is responsible - contributors may touch files without becoming maintainers (Intentionally Opt-In) to maintaining - ownership should survive turnover, refactors, and one-off fixes - git activity is useful as a signal, but weak as a published accountability source ### Manual per-command metadata in source files This was also considered and partially existed already via __author__ in csv2cwms. Reason not chosen: - duplicates data across multiple surfaces - does not scale well across docs, package metadata, and GitHub ownership - becomes harder to maintain consistently over time ### CI-only generation Not chosen. Reason: - CODEOWNERS must exist in the repository for GitHub to use it - committed generated files give deterministic repo state and visible diffs - CI is better used to verify sync, not to be the only place generation happens ## Verification Verified locally with: - python scripts\sync_ownership.py - python scripts\sync_ownership.py --check - poetry run python -m pytest tests\cli\test_all_commands_help.py tests\cli\test_update_command.py -q - python -m sphinx -nW -b html docs docs\_build\html ## Follow-up This covers pages that already exist. For some of the sub commands they do not all have dedicated docs yet.
1 parent f1b631b commit ba722a1

21 files changed

Lines changed: 442 additions & 0 deletions

‎.github/CODEOWNERS‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# DO NOT EDIT THIS FILE MANUALLY.
2+
# Generated from maintainers.toml by scripts/sync_ownership.py.
3+
# Update maintainers.toml, then rerun the sync script.
4+
5+
* charles.r.graham@usace.army.mil eric.v.novotny@usace.army.mil # Default owners for repository-wide files and shared cwms-cli behavior
6+
/.github/CODEOWNERS charles.r.graham@usace.army.mil eric.v.novotny@usace.army.mil # Protect ownership rules themselves
7+
/cwmscli/commands/csv2cwms/ charles.r.graham@usace.army.mil
8+
/docs/cli/csv2cwms*.rst charles.r.graham@usace.army.mil
9+
/cwmscli/commands/blob.py charles.r.graham@usace.army.mil
10+
/docs/cli/blob.rst charles.r.graham@usace.army.mil
11+
/cwmscli/utils/update.py charles.r.graham@usace.army.mil
12+
/docs/cli/update.rst charles.r.graham@usace.army.mil
13+
/cwmscli/load/ charles.r.graham@usace.army.mil eric.v.novotny@usace.army.mil
14+
/docs/cli/load_*.rst charles.r.graham@usace.army.mil eric.v.novotny@usace.army.mil
15+
/cwmscli/usgs/ eric.v.novotny@usace.army.mil
16+
/cwmscli/commands/shef_critfile_import.py eric.v.novotny@usace.army.mil
17+
/cwmscli/commands/commands_cwms.py charles.r.graham@usace.army.mil eric.v.novotny@usace.army.mil # Shared Click wrappers span commands owned by both maintainers

‎.github/workflows/code-check.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,7 @@ jobs:
2222
poetry config virtualenvs.create false
2323
poetry install --no-interaction
2424
25+
- name: Verify generated ownership files
26+
run: poetry run python scripts/sync_ownership.py --check
27+
2528
- uses: pre-commit/action@v3.0.1

‎.pre-commit-config.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,23 @@ repos:
66
entry: poetry run black
77
language: system
88
types: [file, python]
9+
exclude: ^cwmscli/_generated/ownership_data\.py$
910

1011
- id: isort
1112
name: isort
1213
entry: poetry run isort
1314
language: system
1415
types: [file, python]
16+
exclude: ^cwmscli/_generated/ownership_data\.py$
1517

1618
- id: yamlfix
1719
name: yamlfix
1820
entry: poetry run yamlfix
1921
language: system
2022
types: [file, yaml]
23+
24+
- id: ownership-sync
25+
name: generated ownership files
26+
entry: poetry run python scripts/sync_ownership.py --check
27+
language: system
28+
pass_filenames: false

‎CONTRIBUTING.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ Once you have the repository on your system you can proceed:
1515
3. `python -m pip install -e .` - This adds cwms-cli and it's commands to your local path allowing you to live develop cwms-cli as a package and test the CLI functions on your system.
1616
4. Run `cwms-cli` to confirm everything installed!
1717

18+
## Ownership Metadata
19+
20+
Ownership metadata is centralized in [maintainers.toml](/maintainers.toml).
21+
22+
- Run `python scripts/sync_ownership.py` after editing `maintainers.toml` to regenerate `.github/CODEOWNERS` and synced package authors in [pyproject.toml](/pyproject.toml).
23+
- Run `python scripts/sync_ownership.py --check` to verify the generated ownership files are current.
24+
- The local `pre-commit` hook will check this if you installed it, but GitHub Actions is the enforcement point for contributors who do not have hooks installed.
25+
1826
## Running Tests
1927

2028
To run tests you can run: `poetry run pytest`

‎cwmscli/_generated/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Generated files used at runtime and in docs.

‎cwmscli/_generated/ownership_data.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# DO NOT EDIT THIS FILE MANUALLY.
2+
# Generated from maintainers.toml by scripts/sync_ownership.py.
3+
4+
OWNERSHIP_DATA = {
5+
"commands": {
6+
"cwms-cli blob": [
7+
{
8+
"email": "charles.r.graham@usace.army.mil",
9+
"name": "Charles Graham"
10+
}
11+
],
12+
"cwms-cli csv2cwms": [
13+
{
14+
"email": "charles.r.graham@usace.army.mil",
15+
"name": "Charles Graham"
16+
}
17+
],
18+
"cwms-cli load": [
19+
{
20+
"email": "charles.r.graham@usace.army.mil",
21+
"name": "Charles Graham"
22+
},
23+
{
24+
"email": "eric.v.novotny@usace.army.mil",
25+
"name": "Eric Novotny"
26+
}
27+
],
28+
"cwms-cli shefcritimport": [
29+
{
30+
"email": "eric.v.novotny@usace.army.mil",
31+
"name": "Eric Novotny"
32+
}
33+
],
34+
"cwms-cli update": [
35+
{
36+
"email": "charles.r.graham@usace.army.mil",
37+
"name": "Charles Graham"
38+
}
39+
],
40+
"cwms-cli usgs": [
41+
{
42+
"email": "eric.v.novotny@usace.army.mil",
43+
"name": "Eric Novotny"
44+
}
45+
]
46+
},
47+
"default": [
48+
{
49+
"email": "charles.r.graham@usace.army.mil",
50+
"name": "Charles Graham"
51+
},
52+
{
53+
"email": "eric.v.novotny@usace.army.mil",
54+
"name": "Eric Novotny"
55+
}
56+
]
57+
}

‎cwmscli/ownership.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
from __future__ import annotations
2+
3+
from cwmscli._generated.ownership_data import OWNERSHIP_DATA
4+
5+
6+
def _command_candidates(command_path: str) -> list[str]:
7+
parts = command_path.split()
8+
candidates = [" ".join(parts[:i]) for i in range(len(parts), 0, -1)]
9+
return [candidate for candidate in candidates if candidate]
10+
11+
12+
def get_command_maintainers(command_path: str) -> list[dict[str, str]]:
13+
commands = OWNERSHIP_DATA["commands"]
14+
for candidate in _command_candidates(command_path):
15+
if candidate in commands:
16+
return commands[candidate]
17+
return OWNERSHIP_DATA["default"]
18+
19+
20+
def get_core_maintainer_emails() -> set[str]:
21+
return {person["email"] for person in OWNERSHIP_DATA["default"]}
22+
23+
24+
def format_command_maintainers(command_path: str) -> str:
25+
maintainers = get_command_maintainers(command_path)
26+
return ", ".join(person["name"] for person in maintainers)

‎cwmscli/utils/click_help.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import click
77

8+
from cwmscli.ownership import get_command_maintainers, get_core_maintainer_emails
89
from cwmscli.utils import colors
910
from cwmscli.utils.version import get_cwms_cli_version
1011

@@ -60,6 +61,32 @@ def _render_docs_line(ctx: click.Context) -> Optional[str]:
6061
return f"Docs: {colors.c(docs_url, 'blue', bright=True)}"
6162

6263

64+
def _command_path(ctx: click.Context) -> str:
65+
names: list[str] = []
66+
cur: Optional[click.Context] = ctx
67+
while cur is not None:
68+
name = (cur.info_name or getattr(cur.command, "name", None) or "").strip()
69+
if name:
70+
if cur.parent is None:
71+
name = "cwms-cli"
72+
names.append(name)
73+
cur = cur.parent
74+
names.reverse()
75+
return " ".join(names)
76+
77+
78+
def _render_maintainers_line(ctx: click.Context) -> str:
79+
command_path = _command_path(ctx)
80+
core_emails = get_core_maintainer_emails()
81+
rendered = []
82+
for person in get_command_maintainers(command_path):
83+
name = person["name"]
84+
if person["email"] in core_emails:
85+
name = colors.c(name, "blue", bright=True)
86+
rendered.append(name)
87+
return f"Maintainers: {', '.join(rendered)}"
88+
89+
6390
def _inject_help_header(help_text: str, ctx: click.Context) -> str:
6491
lines = help_text.splitlines()
6592
if not lines:
@@ -71,14 +98,21 @@ def _inject_help_header(help_text: str, ctx: click.Context) -> str:
7198

7299
version_line = _render_version_line(ctx)
73100
docs_line = _render_docs_line(ctx)
101+
maintainers_line = _render_maintainers_line(ctx)
74102
if lines[0].startswith("Usage:"):
75103
lines.insert(1, version_line)
76104
if docs_line is not None:
77105
lines.insert(2, docs_line)
106+
lines.insert(3, maintainers_line)
107+
else:
108+
lines.insert(2, maintainers_line)
78109
else:
79110
lines.insert(0, version_line)
80111
if docs_line is not None:
81112
lines.insert(1, docs_line)
113+
lines.insert(2, maintainers_line)
114+
else:
115+
lines.insert(1, maintainers_line)
82116
return "\n".join(lines)
83117

84118

‎docs/_generated/maintainers/blob.inc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.. note::
2+
3+
Maintainers: Charles Graham

‎docs/_generated/maintainers/cli.inc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.. note::
2+
3+
Maintainers: Charles Graham, Eric Novotny

0 commit comments

Comments
 (0)