Attention is currently required from: laforge.

View Change

2 comments:

• File src/osmocom/utils.py:

  • Patch Set #2, Line 141: hexstr

    the "problem" then is that we already have osmocom.utils. […]

    Oh, I forgot that there already exists `Hexstr` type, which is actually here in this file. There's indeed a naming conflict then, and replacing the former with this new class would likely cause hundreds of linting errors. The existing `Hexstr` is just an alias for `str`, which allows an instance of `str` to be passed. If we replace it with this class, we actually force API users to pass an instance of this new class and not `str`...

    I don't like having two classes with the same name and only the upper/lower case making them different, which is definitely confusing. On the other hand, I cannot think of a solution to this problem... Perhaps I should bite the bullet and suggest merging it as-is then.

  • Patch Set #2, Line 151: other

    I was under the impression that the builtin "dunder" method return values are never type-annotated. […]

    I think you're right. We could say `-> bool` for the sake of completeness, but this is not strictly needed. For the `other: str`, I agree that the `str` should work.

To view, visit change 38023. To unsubscribe, or for help writing mail filters, visit settings.