Attention is currently required from: laforge.

neels has posted comments on this change by neels. ( 
https://gerrit.osmocom.org/c/pysim/+/40096?usp=email )

Change subject: personalization: add param_source.py, implement batch 
personalization
......................................................................


Patch Set 10:

(4 comments)

Patchset:

PS6:
> I think we have had this discussion in the osmocom community before, and the 
> general social consensu […]
In simple patches this makes sense, but there is a bound to this:

As a developer I need to be able to *use* the infrastructure that we have 
instead of running my laptop hot, also considering that i cannot run the 
complete pysim test suite on my laptop to begin with.

With a sufficiently complex branch, trying to be considerate about what to push 
becomes unfeasible.

When the gerrit job takes >10 minutes to finish each commit, it must be allowed 
to push ahead and not have to wait on each patch individually.

IMHO we have the [x] Resolved / DONE markings to indicate patch status for 
reviewers.


File pySim/esim/saip/param_source.py:

https://gerrit.osmocom.org/c/pysim/+/40096/comment/4c1deccc_ed242473?usp=email :
PS10, Line 50:
> This is dead code. […]
ok, then yet another ABC cycle.

Ack, with that useless @abstractmethod decorator it is indeed dead code.

We clearly have different approaches. In my opinion, *ABC* is the dead code, 
and you keep advocating it besides the points I make against it -- I have many 
points against it and I find them logical and simple.

Not least that I have spent days exploring the world of ABC, found mostly 
problems it creates, and *one* valid point for it: that developers can read in 
the code that there is inheritance, which a single word comment can do likewise.

So I would very much enjoy if ABC took far less space and time in CR from now 
on.


https://gerrit.osmocom.org/c/pysim/+/40096/comment/059a7954_db20a739?usp=email :
PS10, Line 84:
> likewise here, just `pass` will do.
Done


File pySim/esim/saip/personalization.py:

https://gerrit.osmocom.org/c/pysim/+/40096/comment/af6ada37_3f165569?usp=email :
PS10, Line 705:     def __init__(self,
> highly unusual formatting which we don't do this way in any other part o the 
> code. […]
I am trying hard to read this CR as neutral fact. Thank you for a considerate 
choice of words in CR comments.

I find this formatting readable, because the type annotations clutter up the 
function signature.

For important styling, we should have a linter.

For perspective, I have recently seen a pysim patch with weird spaces in the 
function signature, and including an obvious bug, being +2 and merged within 
minutes. So, what I mean is, I could use any help to get this branch merged.



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/40096?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I497c60c101ea0eea980e8b1a4b1f36c0eda39002
Gerrit-Change-Number: 40096
Gerrit-PatchSet: 10
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Comment-Date: Sun, 15 Feb 2026 17:08:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <[email protected]>
Comment-In-Reply-To: laforge <[email protected]>

Reply via email to