Attention is currently required from: osmith, pespin, dexter.

Patch set 5:Code-Review +2

View Change

1 comment:

• File src/osmo-bsc/pcu_sock.c:

  • Patch Set #1, 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 change 34192. To unsubscribe, or for help writing mail filters, visit settings.