This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
neels gerrit-no-reply at lists.osmocom.orgneels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659 )
Change subject: Add vty interface
......................................................................
Patch Set 2:
(5 comments)
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/Makefile.am
File src/osmo-bsc-nat/Makefile.am:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/Makefile.am@31
PS2, Line 31: vty.c \
I know it's a different source tree, but personally, I prefer if the file name reflects the context: mainly because it is easier to ctags-jump to bsc_nat_vty.c than skip through all the matches for vty.c across all the osmo git source trees... Just mentioning it, it's fine if you prefer this way.
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/logging.c
File src/osmo-bsc-nat/logging.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/logging.c@25
PS2, Line 25: [DMAIN] = {
"DMAIN" seems odd. Usually main() just does fprintf(stderr) or LOGP(DLGLOBAL).
So semantically "main" is not really a subsystem IMO?
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/logging.c@38
PS2, Line 38:
(i usually just put these things in main.c, don't see a reason for a separate logging.c file, especially since that noises up ctags by shadowing the libosmocore logging.c file ... up to you)
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/main.c
File src/osmo-bsc-nat/main.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/main.c@47
PS2, Line 47: .go_parent_cb = bsc_nat_vty_go_parent,
(can drop this, see other comment)
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/vty.c
File src/osmo-bsc-nat/vty.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659/2/src/osmo-bsc-nat/vty.c@31
PS2, Line 31: int bsc_nat_vty_go_parent(struct vty *vty)
you no longer need to define a go_parent or is_config_node callback. Those were needed for the legacy VTY node handling. The new/current vty code implicitly figures out what nodes nest in what other nodes, and also which vty->index object to point to when exiting a child node.
The only reason to implement a go_parent cb is if you want specific actions to happen when the VTY node is exited; imagine that you want to start up or shut down a component when the user is done configuring a node. Usually that's a bad UI design decision though.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26659
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc-nat
Gerrit-Branch: master
Gerrit-Change-Id: I6f2d8d1d62b97be471ebcf1bb14aac0b895833d1
Gerrit-Change-Number: 26659
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: neels <nhofmeyr at sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Dec 2021 13:08:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211228/4f8b4567/attachment.htm>