Skip to content

refactor(utilities): rename hold_warnings to HoldWarnings (#1599) - #1601

Closed
JarryShaw wants to merge 1 commit into
mainfrom
refactor/1599-hold-warnings-rename
Closed

JarryShaw wants to merge 1 commit into
mainfrom
refactor/1599-hold-warnings-rename

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Closes #1599.

Renames pcapkit.utilities.warnings.hold_warnings (#1588, unreleased) to HoldWarnings and drops its # pylint: disable=invalid-name. No alias, since it has never shipped. Touched: pcapkit/utilities/warnings.py, its one caller pcapkit/protocols/protocol.py, docs/source/pcapkit/utilities/{functools,warnings}.rst, and #1588's test file.

HoldWarnings is documented by autoclass and imported from another module, so it is public. It is now listed in pcapkit.utilities.warnings.__all__, beside warn. It was missing there on main too. pcapkit.utilities imports only warn, so neither pcapkit nor pcapkit.all changes.

The pcapkit_held_warnings ContextVar and the test's _HoldNothing stand-in keep their names: neither embeds the class name.

New test test_hold_class_is_named_in_camel_case asserts HoldWarnings imports and is in __all__, and that neither pcapkit.utilities.warnings nor pcapkit.protocols.protocol has hold_warnings. It fails in three cases:

  • on main: ImportError: cannot import name 'HoldWarnings'
  • with a hold_warnings = HoldWarnings alias added: AssertionError: True is not false
  • without the __all__ entry: AssertionError: 'HoldWarnings' not found in ['warn', 'BaseWarning', ...]

Lint: pylint pcapkit/utilities/warnings.py gives 10.00/10 without the disable. On main's copy with the disable removed, it reports C0103: Class name "hold_warnings" doesn't conform to PascalCase naming style. isort is clean. mypy reports nothing in warnings.py.

Tests: I ran narrow selections only. They covered #1588's file, tests/protocols/test_protocol_*_unit.py, tests/test_docstring_contract.py, and the tests/project docs and public-API checks. All passed.

@JarryShaw JarryShaw added refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 10, 2026
`hold_warnings`, added in #1588, was the only lowercase class pcapkit defines
itself, and needed `# pylint: disable=invalid-name`. Rename it to
`HoldWarnings` in `pcapkit/utilities/warnings.py`, its one caller in
`pcapkit/protocols/protocol.py`, the two docs pages and #1588's test file,
and drop the disable. It has not shipped, so no alias is kept.

`HoldWarnings` is documented and imported from another module, so it is
public: list it in `pcapkit.utilities.warnings.__all__`, beside `warn`.

The `pcapkit_held_warnings` ContextVar and the test's `_HoldNothing` stand-in
keep their names: neither embeds the class name.

A new test pins that `HoldWarnings` is importable and exported, and that
neither `pcapkit.utilities.warnings` nor `pcapkit.protocols.protocol` has a
`hold_warnings` attribute.
@JarryShaw
JarryShaw force-pushed the refactor/1599-hold-warnings-rename branch from 5f4a331 to bfac36b Compare October 10, 2026 12:42
@JarryShaw

Copy link
Copy Markdown
Owner Author

Closing unmerged: the owner chose on #1599 to keep the lowercase name. The __all__ export this PR also added moves to #1602, against hold_warnings.

@JarryShaw JarryShaw closed this Oct 10, 2026
@JarryShaw JarryShaw removed the review: running A cross-review is in flight against the current head - no verdict yet label Oct 10, 2026
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

refactor(utilities): rename hold_warnings to HoldWarnings

1 participant