Attention is currently required from: laforge, pespin.
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/27356 )

Change subject: bitvec2freq_list(): fix handling of E-GSM ARFCNs
......................................................................


Patch Set 1:

(4 comments)

Patchset:

PS1:
> this is the kind of issue where a clear-cut unit test showing the bug should 
> be introduced first, an […]
I will add testing coverage in the next patchset version.


PS1:
> I would usually have expected it above the Change-Id

This is actually interesting. In our old patches we used to have 'Related' and 
'Depends' comments *below* the 'Change-Id', but at some point I started to see 
them *above* while doing code review. Maybe the 'commit-msg' hook was updated 
and I missed it?

UPDATE: I checked my .git/hooks/commit-msg, and it indeed differs from 
gerrit.osmocom.org:hooks/commit-msg. The recent version of this hook is based 
on `git interpret-trailers`, while my outdated copy does some grep magic.


File src/osmo-bsc/system_information.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/27356/comment/e47ccfa1_bdd1d204
PS1, Line 508:  && bts->c0->arfcn >= 1 && bts->c0->arfcn <= 124)
             :          pgsm = true;
> I think it's confusing that we have a variable whether or not we are in the 
> PGSM case, but it is tru […]
Ok, I agree. Will rework the patch to do what you suggest.


https://gerrit.osmocom.org/c/osmo-bsc/+/27356/comment/05214e80_8548bcd7
PS1, Line 522:                          /
> I am adding an 'if' statement right after the 'for' loop (see line 532), 
> which checks rc. […]
Marking this thread as resolved.



--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27356
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I17739e6845cd84e2a81bc406dd532541f7c52cb6
Gerrit-Change-Number: 27356
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Wed, 02 Mar 2022 17:22:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <[email protected]>
Comment-In-Reply-To: fixeria <[email protected]>
Gerrit-MessageType: comment

Reply via email to