Max has posted comments on this change. ( https://gerrit.osmocom.org/12761 )
Change subject: Introduce generic host config and related helpers ...................................................................... Patch Set 6: (6 comments) This change is ready for review. https://gerrit.osmocom.org/#/c/12761/5/contrib/jenkins.sh File contrib/jenkins.sh: https://gerrit.osmocom.org/#/c/12761/5/contrib/jenkins.sh@27 PS5, Line 27: > why is this line needed? Done https://gerrit.osmocom.org/#/c/12761/5/src/client.c File src/client.c: https://gerrit.osmocom.org/#/c/12761/5/src/client.c@34 PS5, Line 34: { > can be simplified: #define MATCH(a, b) (strcmp(a,b) == 0) […] Done https://gerrit.osmocom.org/#/c/12761/5/src/client.c@38 PS5, Line 38: switch (k) { > separate lines please. Done https://gerrit.osmocom.org/#/c/12761/5/src/client.c@56 PS5, Line 56: struct host_cfg *cfg = talloc_zero(ctx, struct host_cfg); > This is basically a constructor/allocator, so better name it like we usually > do, like talloc_host_cf […] Done https://gerrit.osmocom.org/#/c/12761/5/src/client.c@79 PS5, Line 79: > It's allocating something and the method is not const, so I'd better name it > "create_authority" or " […] Done https://gerrit.osmocom.org/#/c/12761/5/src/client.c@81 PS5, Line 81: > Is this really needed? Yes, it's easier to use it that way because it's used by both logging and polling code. -- To view, visit https://gerrit.osmocom.org/12761 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sysmon Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie321655a92cdbefbfaa056ac0d583397c83beccb Gerrit-Change-Number: 12761 Gerrit-PatchSet: 6 Gerrit-Owner: Max <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Thu, 07 Feb 2019 16:41:15 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
