Attention is currently required from: pespin, fixeria. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/27644 )
Change subject: rsl: Fix tlv_parse of IPAC_DLCX_IND message ......................................................................
Patch Set 1:
(3 comments)
File tests/abis/abis_test.c:
https://gerrit.osmocom.org/c/libosmocore/+/27644/comment/2b2de44c_60f2dde4 PS1, Line 196: SYS#5915
Same here.
I think it's fine as used here, the comment makes sense even without (being able to) reading the issue, but it can be useful if somebody wants the additional context.
https://gerrit.osmocom.org/c/libosmocore/+/27644/comment/c225a99e_363a7e7e PS1, Line 235: 5891 * different issue number? * I'd also not mention it here, having it once in the comment makes more sense IMHO
https://gerrit.osmocom.org/c/libosmocore/+/27644/comment/a9c14de7_52bc27f9 PS1, Line 251: sys5915
It is still fine to add a suffix to it in case we want to test decoding same message type using diff […]
Using tickets in the function names seems weird to me. Even if one has access to it, it's not meaningful until looking up the ticket. I'd suggest using test_dec_ipac_dlc_indx() instead, and if we want to do additional tests like you said, then find another (hopefully more descriptive) suffix for those.