nim-lang / RFCs

A repository for your Nim proposals.
135 stars 26 forks source link

testament: add default target which honors `NIM_COMPILE_TO_CPP` #334

Open timotheecour opened 3 years ago

timotheecour commented 3 years ago

proposal 1

proposal 2

replace all instances of targets: c ... with s/c/default/ (except those for which somethings breaks) as follows:

before: targets: "c js"

after: tests/stdlib/turi.nim:2:3: targets: "default js"

for things like: tests/system/tdollars.nim:2:3: targets: "c cpp js" it may make sense to keep as is so that all backends still get tested in this case even if there is some redundancy (NIM_COMPILE_TO_CPP only is used in 1 osx instance in CI at the moment)

proposal 3

make megatest honor NIM_COMPILE_TO_CPP (right now it hardcodes to c) possibly other tests too

proposal 4

nit: rename NIM_COMPILE_TO_CPP: true|[false] to NIM_TESTAMENT_TARGET: cpp|js|objc|[c] which has more useful semantics

benefits

refs https://github.com/nim-lang/Nim/pull/16820#issuecomment-772087369

juancarlospaco commented 3 years ago

I dont understand the title, like tries to say too much on too few words.

I dont understand the "alias", like whats the benefit of "default" over "c" ?, I prefer explicit settings.

I like the idea of the possibility of settings via env vars, but I think if you really taking the time to work on it, better add .env support to stdlib with compile-time type-safe configs and can be reused when needed, .env is not a very complex algo, at least as a /private/ module, should not take more than 1 file at most.

timotheecour commented 3 years ago

I dont understand the title, like tries to say too much on too few words.

i changed title

I dont understand the "alias", like whats the benefit of "default" over "c" ?, I prefer explicit settings.

targets: "c js" won't honor NIM_COMPILE_TO_CPP in CI, so you lose test coverage for cpp backend

I like the idea of the possibility of settings via env vars, but I think if you really taking the time to work on it, better add .env support to stdlib with compile-time type-safe configs and can be reused when needed, .env is not a very complex algo, at least as a /private/ module, should not take more than 1 file at most.

NIM_COMPILE_TO_CPP is already used in testament; it's just not honoring certain tests, eg: