Patch Set 3: Code-Review-1 (7 comments)
https://gerrit.osmocom.org/#/c/2831/3//COMMIT_MSG Commit Message: Line 17: This patch adds a function that can be used as a helper to fill (generally in commit logs, rather use the imperative form: "Add function foo()." It is also good practice to put relevant information in the API doc and limit the commit log to reasons / decision process) https://gerrit.osmocom.org/#/c/2831/3/src/gsm/gsm0808_utils.c File src/gsm/gsm0808_utils.c: Line 701: /* Extrapolate a speech codec field from a given permitted speech parameter */ Need proper API doc, as you will see for other functions now committed on master. Line 707: * codec that field that consistantly matches the channel type "that field that"? "consistently" Line 708: * configuration. This will basically reflect a non-transcoding- "This reflects a" Line 710: * stream may be differ from the codec used on the air interface) */ "may differ from" seems to me this comment belongs in the API doc Line 722: /* Depending on the, pick a default codc configuration, that on the? No comma before "that" Line 743: } missing "default:" case to return -EINVAL? -- To view, visit https://gerrit.osmocom.org/2831 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I257c972e9fdf0dfe940a8d483447085bd62e50a2 Gerrit-PatchSet: 3 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-HasComments: Yes
