Attention is currently required from: neels. fixeria has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email )
Change subject: add VTY 'gtp-echo' command ...................................................................... Patch Set 1: Code-Review-1 (3 comments) File src/osmo-upf/upf_vty.c: https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/00357114_5cc25bcb?usp=email : PS1, Line 491: tx to The command vector looks overloaded to me: * `tx to` looks redundant, you could just say `gtp-echo REM_ADDR`? * `[from]` looks weird, maybe `(local-ip|local-dev)` instead? * how about using `VTY_IPV46_CMD`? I propose defining the following command vectors separately: * `gtp-echo REM_ADDR` (brief and simple) * `gtp-echo REM_ADDR local-ip VTY_IPV46_CMD` * `gtp-echo REM_ADDR local-dev DEV_NAME` Yes, this is indeed more code, but would be cleaner IMO. https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/f8192400_3f56cab1?usp=email : PS1, Line 556: %%Error cosmetic: missing space File tests/gtp-echo.vty: https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/8bc12cc5_960bbfce?usp=email : PS1, Line 2: show pid : show uptime : show version : show online-help : list [with-flags] : exit : help : show vty-attributes : show vty-attributes (application|library|global) : enable [expert-mode] : terminal length <0-512> : terminal no length : these lines (and more below), for obvious reasons, shall not be part of the VTY test -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I970dccd7a27b098eea9e660822e24e2c4b059fc6 Gerrit-Change-Number: 37830 Gerrit-PatchSet: 1 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Comment-Date: Sat, 17 Aug 2024 14:47:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
