AbsaOSS / AbsaOSS/EventGate

Remove __all__ declarations from src/utils modules

オープン
#221 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
refactoring type:tech-debt
主要言語
Python
スター
4
フォーク
0
平均マージ
1日 5時間
マージ済み PR(30日)
8

説明

### Description of Technical Debt

Five modules under `src/utils/` end with an `__all__` declaration:

| Module | Line | Names |
|---|---|---|
| `src/utils/conf_path.py` | 90 | 4 |
| `src/utils/logging_levels.py` | 71 | 3 |
| `src/utils/observability.py` | 217 | 16 |
| `src/utils/safe_serialization.py` | 87 | 1 |
| `src/utils/trace_logging.py` | 53 | 1 |

`__all__` controls exactly one thing: which names `from module import *` binds. There is no `import *` anywhere in `src/` or `tests/`, and explicit imports are the convention the codebase already follows everywhere. The declarations therefore have no effect on any current call site.

Raised in review on PR #204 and agreed by both reviewer and author.

### Impact of Technical Debt

- **Maintenance with no return.** Every new public name in these modules means editing a second list that nothing reads. The lists are in sync today, so the cost so far has been paid entirely in review attention rather than caught bugs.
- **Misleading signal.** A reader encountering `__all__` reasonably infers that star imports are expected somewhere, or that the module has a curated public surface distinct from its non-underscore names. Neither is true.
- **Inconsistent with the rest of the codebase.** No other package in the project declares `__all__`, and it is not a pattern used in sibling Python projects, so it reads as local convention where there is none.

### Category

Code Quality / Refactoring

### Priority

Low - Nice to have

### Proposed Solution

Delete the `__all__` block from all five modules. Rely on the existing convention: a leading underscore marks a name private, everything else is importable.

Two things to confirm while doing it, both currently safe:

1. **Re-exports.** `src/utils/observability.py` lists `TRACE_LEVEL` and `configured_log_level`, which it imports from `src/utils/logging_levels.py` rather than defining; `src/utils/conf_path.py` similarly lists `CONF_DIR` and `INVALID_CONF_ENV`. Under mypy's `no_implicit_reexport` an imported name is only re-exported if it appears in `__all__` or is aliased. The project's mypy config (`pyproject.toml [tool.mypy]`) does **not** enable `strict` or `no_implicit_reexport`, so removal changes nothing — but if that config is tightened later, importers of those names through `observability` would need to import them from `logging_levels` instead.
2. **No star imports.** `grep -rn "import \*" --include=*.py src/ tests/` returns nothing. Re-run before merging in case one has been added.

### Effort Estimate

< 1 hour

### Dependencies / Related

- PR #204, where this was raised
- #193

### Additional Context

Acceptance:

- No `__all__` remains under `src/`.
- `pytest tests/unit`, `mypy`, `pylint`, and `black --check` all pass unchanged.
- No import statement anywhere needs editing, which is the point: if one does, that name was relying on a re-export and should be imported from its defining module.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。