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 Hofmeyr gerrit-no-reply at lists.osmocom.orgNeels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11836 ) Change subject: chapters/gsup.adoc: document MO-/MT-forwardSM messages ...................................................................... Patch Set 2: Code-Review-1 (12 comments) I'm sorry to criticise as much as this. I started out wanting to just +2 straight away, but ended up finding numerous trivial tweaks that would greatly serve readability. The bottom line is, this is probably correct, but I am distracted by the writing style ;) https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc File common/chapters/gsup.adoc: https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@507 PS2, Line 507: ==== MO-forwardSM Request I'm slightly critical of the mix of dash and non-caps / CamelCase / acronym style. Why not just "MO Forward Short Message Request" -- that is easy to understand for the reader on first sight. https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@509 PS2, Line 509: Direction: SGSN / MSC => ESME (through VLR and HLR) I'm curious why you name the SGSN first. So far I wasn't aware of our SGSN even able to send SMS. https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@522 PS2, Line 522: This message is used to forward _MO (Mobile Originated)_ short messages would be nice to first say it in simple rather nontechnical terms and then add details. Like, "When a subscriber sends an SMS, this message forwards it to the ESME. [...]" https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@524 PS2, Line 524: Entity). The corresponding MAP service (defined in 3GPP TS 29.002) is We have a glossary of acronyms, maybe rather make sure they are in the glossary and abbreviate everywhere / choose to write them out everywhere. Naming both makes it harder to follow the sentence flow. Same goes for indicating "MS (CS domain) / SGSN (PS domain)" all the time, this is not the place to explain what PS and CS mean, so rather assume the reader knows these acronyms. https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@525 PS2, Line 525: *MAP-MO-FORWARD-SHORT-MESSAGE*, described in section 12.2. rather give the spec reference once, complete, in the end "... (see 3GPP TS 29.002 section 12.2)" https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@528 PS2, Line 528: attempt and _RP-SMMA_ notification attempt on the _SM-RL (Relay Layer)_. "attempt" sounds like it fails. "request"? https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@532 PS2, Line 532: Direction: ESME (through VLR and HLR) => SGSN / MSC above it spells "MSC => ESME (through VLR and HLR)" -- maybe rather reverse it here? "ESME (through HLR and VLR) -> MSC"? https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@545 PS2, Line 545: short message delivery from SGSN (PS domain) / MSC (CS domain) to an ESME above it says ESME => MSC, in this sentence the direction is reversed? Copy-paste error? Or did you mean that the earlier message was in the other direction? To avoid the confusion, you could simply not name the direction again, just rely on the reader looking at the direction definition on top and knowing that an error response is for an earlier request in the other direction. https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@546 PS2, Line 546: (External Short Message Entity). Thus it triggers an _RP-ERROR_ on the _SM-RL this is not the place to define what should happen in the implementation, is it? If at all, this could indicate "should usually trigger", but foremost, this explains the message definition, and the semantics are up to the implementation / explained elsewhere. https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@1094 PS2, Line 1094: Relay Layer)_ has an unique _message reference (see 8.2.3)_, that Don't ask me why, but it is "a unique". I know the consonant vs vocal rule, but that is not as deterministic as one might think. There is also "an RSL procedure" because "R" is spoken "arr"... 8.2.3 in this document? then use a <<foo>> link. 8.2.3 in 04.11? then write the full '3GPP TS 04.11 section 8.2.3' out here, but rather not break the flow of the sentence with it. I'd not use italics at all. If they are "given names", capitalize. If already capitalized, italics are redundant. If you really must use italics, don't also use italics for '(see 8.2.3)'. (I used to use a lot of markup too in the past, but now my opinion is they are too much effort and rather than helping, they clutter up the reading flow.) https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@1110 PS2, Line 1110: * IMSI (see clause 7.6.2.1); clause 7.6.2.1 *where*? https://gerrit.osmocom.org/#/c/11836/2/common/chapters/gsup.adoc@1111 PS2, Line 1111: * LMSI (see clause 7.6.2.16, not implemented!); clarify "not implemented" -- this here should be a protocol definition: it is by definition implemented or not implemented by the *implementations* (libosmo-gsup-foo), this protocol definition cannot be "not implemented" by itself. -- To view, visit https://gerrit.osmocom.org/11836 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-gsm-manuals Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0150756c33c1352bc4eb49421824542c711175c Gerrit-Change-Number: 11836 Gerrit-PatchSet: 2 Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Comment-Date: Mon, 19 Nov 2018 23:25:12 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181119/b68e83a3/attachment.htm>