TangMeng12 commented on code in PR #17236:
URL: https://github.com/apache/nuttx/pull/17236#discussion_r2469357570
##########
Kconfig:
##########
@@ -264,6 +264,14 @@ config APPS_DIR
example, to include makefile fragments (e.g., .config or
Make.defs)
or to set up include file paths.
+config ALLOW_DOWNLOADS
Review Comment:
> If we're adding this option, it should work for ALL downloads in NuttX,
not just ESP chips. Otherwise, it's incomplete feature and very confusing for
users because it'll only work for a tiny part of the project.
Because so far it has only been found that Espressif needs to download the
git repository, and there is no logic set to skip downloading. So here I first
added this option and adapted Espressif's corresponding code.
For mostly in the nuttx-apps repo, it checks whether the `.git` directory is
exist, and if it exist, it will not automatically download external packages
from the Internet.
The original intention of this PR is to add a config
`CONFIG_ALLOW_DOWNLOADS` that can be adapted to any other place in the future,
rather than only for the Espressif logic here.
I think providing multiple ways to determine whether a download is needed is
more reliable than directly replacing the logic everywhere. If we later want to
adapt all the code to use `CONFIG_ALLOW_DOWNLOADS`, we can gradually delete and
replace the various download logics in other places.
This modification can be divided into two schemes:
1. Only for Espressif, define a `CONFIG_ESP_ALLOW_DOWNLOADS`, which only
applies to ESP.
2. Add `CONFIG_ALLOW_DOWNLOADS`, gradually adapting code in other areas,
with all automatic downloads controlled under `CONFIG_ALLOW_DOWNLOADS`(I think
a gradual replacement process is more appropriate, rather than switching
everything all at once in this PR).
--
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]
