Attention is currently required from: laforge.

fixeria has posted comments on this change by laforge. ( 
https://gerrit.osmocom.org/c/python/pyosmocom/+/38023?usp=email )

Change subject: Introduce a new 'hexstr' type to represent hex-strings
......................................................................


Patch Set 2:

(2 comments)

File src/osmocom/utils.py:

https://gerrit.osmocom.org/c/python/pyosmocom/+/38023/comment/e1b3545e_9703645d?usp=email
 :
PS2, 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.


https://gerrit.osmocom.org/c/python/pyosmocom/+/38023/comment/3f4fd944_8b39fff5?usp=email
 :
PS2, 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 https://gerrit.osmocom.org/c/python/pyosmocom/+/38023?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: python/pyosmocom
Gerrit-Branch: master
Gerrit-Change-Id: I16c0df809bc11ec0f98e8ade404f9b82072e3a06
Gerrit-Change-Number: 38023
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: Jenkins Builder
Gerrit-Attention: laforge <[email protected]>
Gerrit-Comment-Date: Thu, 05 Sep 2024 10:43:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <[email protected]>
Comment-In-Reply-To: fixeria <[email protected]>

Reply via email to