On 05/02/2017 12:27 PM, Dave Thaler via iotivity-dev wrote:
> I just added the following guidance to the Platform Support project coding
> guidelines (https://wiki.iotivity.org/platform_support):
> Use of target_os in SConscript files
> Often there are things that do not apply to all target OS's and so an
> SConscript file needs to put some functionality under an 'if' test against
> target_os. The following applies when deciding when to use an inclusion test
> (i.e., "==" or "in") vs an exclusion list (i.e., "!=" or "not in"):
> 1. Use exclusion when the functionality ought to apply but doesn't yet
> because some porting work has not yet been done.
> 2. Use inclusion when the functionality is inherently specific to one or
> more specific OS's
> If there are comments, please discuss here. Otherwise, please follow the
> above in all new changes to SConscript files.
> I believe this same principle is what is already followed in other OSS
> projects including libcoap which we consume.
>
> Dave
This sounds sane.
I'd like to also add that there are a lot of tests which end up with
platform specifics in a way that changing platform details can make it
very cumbersome to apply, or even try out, the change. If 100
sconscripts do something, it's a pain to go through them all, and it
pretty much has to be done manually.
So keep an eye on whether the test needs to be local or should be set in
build_common to apply everywhere. For example:
if target_os not in ['windows']:
rd_sample_app_env.AppendUnique(CXXFLAGS = ['-O2', '-g', '-Wall',
'-Wextra', '-std=c++0x'])
thost look like global flags, but I fished it out of a local sconscript
under resource/csdk/resource-directory
or tests like this:
if target_os in ['linux', 'windows']:
run_test(.....
here I'd rather see a check against a globally set/exported variable,
say targets_support_tests, so turning on a new platform's ability to run
tests doesn't require editing everywhere; then if there are tests that
should not be run for a specific platform because they don't work yet,
they can be individually excluded until they're ported.
And in the end: when a target is included/excluded in a check, it should
be clear why. "Tribal knowledge" is nice but isn't friendly to new
developers.
