[riot-notifications] [RIOT-OS/RIOT] boards: add support for Phytec 'reel board' (#11096)

Alexandre Abadie notifications at github.com
Tue Mar 5 20:22:12 CET 2019


aabadie commented on this pull request.

I have a few minor comments, even if it looks good in general code-wise. I haven't the board for testing but you can easily find someone in Berlin for this.

> @@ -0,0 +1,14 @@
+/**
+ at defgroup    boards_reel reel
+ at ingroup     boards
+ at brief       Support for the PHYTEC 'reel board'
+
+## Overview
+
+The 'reel board' is an IoT development platform base on Nordic's nRF52840 SoC.

s/base on/based on/

> @@ -0,0 +1,14 @@
+/**
+ at defgroup    boards_reel reel

The board group name should be more explicit, e.g. something like `Phytec reel board` or something. Otherwise, this will just add an entry called 'reel' in the doxygen documentation, this will look weird.

Regarding the build system, I know 'reel' is fast and simple to type when calling make, but it's also a too generic word. What about using `phytec-reel` ? Like this it will be listed close to other phytec boards (pba-xxx-zz-kw2x and phynode-kw41z).
This second comment is not blocking, and after all, you are the author, so you decide ;)

-- 
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/11096#pullrequestreview-210869630
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190305/9fb1aae2/attachment-0001.html>


More information about the notifications mailing list