[riot-notifications] [RIOT-OS/RIOT] [WIP] cpu/atmega_common: pseudomodule-based pin change interrupt implementation (#11122)

Marian Buschsieweke notifications at github.com
Fri Mar 8 10:57:05 CET 2019


maribu commented on this pull request.

Two more things (sorry for the seamlessly endless string of nit-picking):

1. Some files do not have a newline at the end, the CI will complain about that. (I think it should be exactly one new line to keep them happy, as they will complain about two and more es well.)
2. See inline comment

> @@ -3,3 +3,5 @@ USEMODULE += boards_common_atmega
 ifneq (,$(filter saul_default,$(USEMODULE)))
   USEMODULE += saul_gpio
 endif
+
+-include $(RIOTCPU)/$(CPU)/Makefile.dep

I think this is better placed in `boards/common/atmega/Makefile.dep`, as not all AVR based boards are Arduinos. (And this and every non-Arduino AVR board should then include `$(RIOTBOARD)/common/atmega/Makefile.dep`).

Also, I would prefer to use `include $(RIOTCPU)/$(CPU)/Makefile.dep` instead of `-include` here, so that developers adding support for new ATmega CPUs will get an error if they did not provide a corresponding `Makefile.dep`.

-- 
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/11122#pullrequestreview-212211360
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190308/3ee874d5/attachment-0001.html>


More information about the notifications mailing list