w8jcik commented on a change in pull request #334:
URL:
https://github.com/apache/incubator-nuttx-apps/pull/334#discussion_r455143417
##########
File path: graphics/lvgl/Makefile
##########
@@ -78,20 +78,21 @@ $(LVGL_UNPACKNAME): $(LVGL_TARBALL)
$(Q) mv lvgl-$(LVGL_VERSION) $(LVGL_UNPACKNAME)
$(Q) touch $(LVGL_UNPACKNAME)
-lvgl/lvgl.h: $(LVGL_UNPACKNAME)
+$(LVGL_UNPACKNAME)/lvgl.h: $(LVGL_UNPACKNAME)
-$(APPDIR)/include/graphics/lvgl.h: lvgl/lvgl.h
- @echo "CP: lvgl/lvgl.h"
- $(Q) cp lvgl/lvgl.h $(APPDIR)/include/graphics/lvgl.h
+exports/graphics/lvgl.h: $(LVGL_UNPACKNAME)/lvgl.h
+ $(Q) mkdir -p exports/graphics
+ @echo "CP: $(LVGL_UNPACKNAME)/lvgl.h"
+ $(Q) cp $(LVGL_UNPACKNAME)/lvgl.h exports/graphics/lvgl.h
Review comment:
Hi @v01d the prefix is already present, because currently littlevgl is
copying some header to `include/graphics` to be consistent with all NuttX apps
libraries. Change suggested by @xiaoxiang781216 removes this prefix. I was
asking should we really remove it, because this way we break compatibility.
Breaking compatibility is fine for me. But what is the official strategy. Do we
want `graphics` in front of all lvgl includes? Right now it seems to be in
front of one of the headers and probably not in front of the others. I think it
should be still possible to have `#include <graphics/lvgl/...>`. The question
is do we want it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
