AbsaOSS / AbsaOSS/EventGate

Remove __all__ declarations from src/utils modules

未關閉
#221 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
refactoring type:tech-debt
主要語言
Python
星號
4
分支
0
平均合併
1 天 5 小時
30 天內合併 PR
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 摘要。