mlyszczek commented on PR #3477:
URL: https://github.com/apache/nuttx-apps/pull/3477#issuecomment-4475023811

   > @simbit18 I am sorry, but the nuttx-apps I used for inspiration 
(`system/psmq` and `logging/embedlog`) do not include CMakeLists.txt either. I 
am not sure how to write CMakeLists.txt for rtttl-c.
   
   Hi just a note here, I've wrote rules for psmq and embedlog in times where 
cmake was were it belonged (nowhere near nuttx-apps).
   
   That being said I would have no idea how to do those in cmake now either. 
It's fairly easy to do arbitrary steps with makefiles like download a file, sed 
what needs to be seded etc. Nuttx would have to implement easy to use cmake 
functions for such cases if it's expected from devs to add cmake support for 
3rd party apps downloaded from the net. And IMO since nuttx is posix, it should 
be very important to allow for easy 3rd party app integration - like it's done 
with buildroot. But that's just my opinion.
   
   ---
   So I don't come as a whiner without merit, I will give you a review too :)
   
   First use known tag instead of getting file from the master when clonning. 
Nothing worse than having program working today, and being broken randomly 
tomorrow because upstream did some changes. If you need master, then you could 
add version selector to kconfig.
   
   Second, nn download step I would add `$Q $(NXTOOLSDIR)/check-hash.sh sha256 
$(RTTTL_SRC_SHA256) $@/rtttl.c` and keep hash in makefile statically. Things 
get hacked, you don't want someone to poison your project with malware because 
some 3rd party lib you are using was careless.


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to