Attention is currently required from: fixeria, laforge.

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455?usp=email )

Change subject: msc: f_expect_paging(): fix by_tmsi arg
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

File msc/BSC_ConnectionHandler.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36455/comment/2d5f24d7_0be16b88
PS2, Line 1360: function f_expect_paging(template OCT4 tmsi := *)
oh, i expected that it would need to adjust lots of callers...


Not blocking; But this patch breaks one paradigm of this function: see below 
how we use g_pars.imsi.
Now this is not using g_pars.tmsi as would be logical, but the caller is 
passing a TMSI explicitly.
This is the reason why i all the time and still and always will think a boolean 
is semantically the right choice. To fix it, you'd also have to make the imsi 
an argument that is passed in and not taken from g_pars.

But since someone spent time to submit the patch and it's good enough, I'll not 
make their life harder with my fringe opinion on the far corner of a ttcn3 
test's helper function's type of an arg, and will accept the patch despite not 
matching my opinion.

I'd like to see more of that, please, very large wink.



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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I9434745b7faeb738caafed8080b9f7b1a6a8079a
Gerrit-Change-Number: 36455
Gerrit-PatchSet: 2
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-CC: laforge <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Fri, 12 Apr 2024 22:29:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Reply via email to