Skip to content

Typing readelf and dwarfdump - #657

Merged
eliben merged 2 commits into
eliben:mainfrom
pmhahn:typing-other
May 26, 2026
Merged

Typing readelf and dwarfdump#657
eliben merged 2 commits into
eliben:mainfrom
pmhahn:typing-other

Conversation

@pmhahn

@pmhahn pmhahn commented May 26, 2026

Copy link
Copy Markdown
Contributor

The last bits from #611 are extra as they do not type the API, but the scripts/. I have used this to develop the type-hints for the API, but this is not strictly required unless you consider them part of the API.

I find them very useful to understand, how the API is supposed to use. As some of their functions are large and complex, having some hint about the types being passed between those functions helps to understand their use.

Typing is not complete and scripts/ is currently not checked: the path is missing in pyproject.toml.

  1. The path cannot simply be added as both scripts define a main() function. Checking both files would required converting them into modules so they can be imported with different names.
  2. There are many locations in both files, which requires more assert isinstance(…) or assert … is not None to be clean. IMHO that would clutter the code too much

If one or both is desired, I can have another look.

pmhahn added 2 commits May 26, 2026 05:14
Add the easy type annotations to have some test when running with
typeguard enabled.

Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
Add the easy type annotations to have some test when running with
typeguard enabled.

Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
This was referenced May 26, 2026
@eliben

eliben commented May 26, 2026

Copy link
Copy Markdown
Owner

Thanks, I'm OK with these minimal additions but agree that doing large changes just for the sake of typing these is overkill. These scripts are mainly used for testing.

@eliben
eliben merged commit c6cb7d7 into eliben:main May 26, 2026
5 checks passed
@pmhahn
pmhahn deleted the typing-other branch May 26, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants