[riot-notifications] [RIOT-OS/RIOT] sys: Add Link Format module (#11189)

Sebastian Meiling notifications at github.com
Wed Jun 12 10:04:36 CEST 2019


smlng requested changes on this pull request.

first review round mostly on documentation, there should also be a reference to RFC 6690 which this is implementing

> @@ -0,0 +1,3 @@
+MODULE = clif

not needed, module name is autogenerated from path in `sys` 

> +#define LF_LINK_SEPARATOR_C ','
+
+/**
+ * @brief link format parameter separator character
+ *
+ */
+#define LF_PARAM_SEPARATOR_C ';'
+
+/**
+ * @brief link format attribute separator character
+ */
+#define LF_PARAM_ATTR_SEPARATOR_C '='
+
+/**
+ * @{
+ * @brief link format anchor parameter string and size

this grouping looks odd, is this correct doxygen? IIRC grouping should use `@name` instead of `@brief` and should like:

```
/**
 * @name link format anchor parameter string and size
 * @{
 */
#define LF_PARAM_ANCHOR      "anchor"  /**< param name */
#define LF_PARAM_ANCHOR_S    (6)       /**< param name length */
/** @} */
```

> +#define LF_LINK_SEPARATOR_C ','
+
+/**
+ * @brief link format parameter separator character
+ *
+ */
+#define LF_PARAM_SEPARATOR_C ';'
+
+/**
+ * @brief link format attribute separator character
+ */
+#define LF_PARAM_ATTR_SEPARATOR_C '='
+
+/**
+ * @{
+ * @brief link format anchor parameter string and size

I think the extra documentation on each define is optional

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11189#pullrequestreview-248596120
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190612/8c5288a6/attachment-0001.html>


More information about the notifications mailing list