<p></p>
<p><b>@fjmolinas</b> commented on this pull request.</p>

<p>Some testing where I removed the <code>external_modules</code> directory in <code>tests/external_module_dirs</code> and put it outside of tree and exported the location:</p>
<details> <summary> compile test <b>OK</b> </summary>
<pre><code>make -C tests/external_module_dirs/
Building application "tests_external_module_dirs" for "native" with MCU "native".

"make" -C /home/francisco/workspace/RIOT/boards/native
"make" -C /home/francisco/workspace/RIOT/boards/native/drivers
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/native
"make" -C /home/francisco/workspace/RIOT/cpu/native/periph
"make" -C /home/francisco/workspace/RIOT/cpu/native/stdio_native
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/luid
"make" -C /home/francisco/workspace/RIOT/sys/random
"make" -C /home/francisco/workspace/tmp/external_modules/external_module
   text    data     bss     dec     hex filename
  29345     628   47820   77793   12fe1 /home/francisco/workspace/RIOT/tests/external_module_dirs/bin/native/tests_external_module_dirs.elf
</code></pre>
</details>
<details> <summary> compile test in docker <b>OK</b> </summary>
<pre><code>Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/home/francisco/workspace/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'   -v '/home/francisco/workspace/tmp/external_modules:/data/riotbuild/external/external_modules:delegated'      -w '/data/riotbuild/riotbase/tests/external_module_dirs/' 'riot/riotbuild:latest' make  'EXTERNAL_MODULE_DIRS=/data/riotbuild/external/external_modules /data/riotbuild/riotbase/tests/external_module_dirs/external_modules'    
Building application "tests_external_module_dirs" for "native" with MCU "native".

"make" -C /data/riotbuild/external/external_modules/external_module
"make" -C /data/riotbuild/riotbase/boards/native
"make" -C /data/riotbuild/riotbase/boards/native/drivers
"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/cpu/native
"make" -C /data/riotbuild/riotbase/cpu/native/periph
"make" -C /data/riotbuild/riotbase/cpu/native/stdio_native
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/sys/auto_init
"make" -C /data/riotbuild/riotbase/sys/luid
"make" -C /data/riotbuild/riotbase/sys/random
   text    data     bss     dec     hex filename
  28792     628   47756   77176   12d78 /data/riotbuild/riotbase/tests/external_module_dirs/bin/native/tests_external_module_dirs.elf
</code></pre>
</details>
<details> <summary> compile test with setup matching previous handling <b>FAIL</b> </summary>
<pre><code>BUILD_IN_DOCKER=1 make -C tests/external_module_dirs/
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/home/francisco/workspace/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'   -v '/home/francisco/workspace/tmp/external_module:/data/riotbuild/external/external_module:delegated'      -w '/data/riotbuild/riotbase/tests/external_module_dirs/' 'riot/riotbuild:latest' make  'EXTERNAL_MODULE_DIRS=/data/riotbuild/external/external_module /data/riotbuild/riotbase/tests/external_module_dirs/external_modules'    
Building application "tests_external_module_dirs" for "native" with MCU "native".

/data/riotbuild/riotbase/tests/external_module_dirs/main.c:23:10: fatal error: external_module.h: No such file or directory
 #include "external_module.h"
          ^~~~~~~~~~~~~~~~~~~
compilation terminated.
/data/riotbuild/riotbase/Makefile.base:107: recipe for target '/data/riotbuild/riotbase/tests/external_module_dirs/bin/native/application_tests_external_module_dirs/main.o' failed
make[1]: *** [/data/riotbuild/riotbase/tests/external_module_dirs/bin/native/application_tests_external_module_dirs/main.o] Error 1
/data/riotbuild/riotbase/Makefile.include:641: recipe for target 'application_tests_external_module_dirs.module' failed
make: *** [application_tests_external_module_dirs.module] Error 2
make: *** [/home/francisco/workspace/RIOT/makefiles/docker.inc.mk:314: ..in-docker-container] Error 2
</code></pre>
</details>
<p>So the thing that remains IMO is how to notify external users of the change, since the variable name has not changed it's hard to add a warning. Too bad the current name is quite good already.... I would suggest adding a warning if <code>EXTERNAL_MODULE_DIRS</code> is set for a release or two, what do you think <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/users/maribu/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/maribu">@maribu</a>?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/16104#pullrequestreview-650998841">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYGNQT4AI5QQ4NFSHOLTL6WFJANCNFSM4YI3OCGQ">unsubscribe</a>.<img src="https://github.com/notifications/beacon/ABE7WYAWWCJZO6K7TD2JEJ3TL6WFJA5CNFSM4YI3OCG2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOE3GXIOI.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/RIOT-OS/RIOT/pull/16104#pullrequestreview-650998841",
"url": "https://github.com/RIOT-OS/RIOT/pull/16104#pullrequestreview-650998841",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>