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 LVGL wrapper
is copying some header to `include/graphics` to be consistent with other NuttX
apps libraries. Change suggested by @xiaoxiang781216 removes this prefix. I was
asking should we really remove it, because this way we break compatibility and
violate consistency. 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]
