Attention is currently required from: osmith, pespin, dexter.
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34192 )
Change subject: pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34192/comment/24b27257_6553e915
PS1, Line 552: if (pch->confirmed_imm_ass)
I disagree with this. […]
One can already see
the consequences of making breaking changes "on the go" without bumping the
protocol version: some testcases in ttcn3-bts-test started to fail because the testsuite
is recent enough and including the `confirm` field, while current osmo-bts is not recent
enough and rejecting DATA.req as malformed.
https://jenkins.osmocom.org/jenkins/view/TTCN3/job/ttcn3-bts-test/2135/ (+8 failures)
Once this and the respective osmo-{pcu,bts} changes are merged, recent osmo-pcu speaking
PCUIFv11 will become incompatible with older osmo-{bsc,bts} speaking the same [but
different] PCUIFv11. And vice-versa.
As I said in
https://gerrit.osmocom.org/c/osmo-bts/+/34059/comments/5a247c05_3683d840, I
consider this a bad practice because this makes life harder for those using nightly and
even harder for those running git-bisect. Protocol changes like this (and preceding ones)
should be carefully planned, discussed, and merged all in one shot followed by a version
bump. No breaking changes should be made after the version bump.
Don't take this as blaming or shaming, all I want to say is that we should avoid such
situations in the future when changing these ad-hoc protocols. The patch itself looks good
to me.
--
To view, visit
https://gerrit.osmocom.org/c/osmo-bsc/+/34192
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
Gerrit-Change-Number: 34192
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 30 Aug 2023 22:42:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment