[x] using nim apis (instead of shelling out) is preferred, making code more portable and robust, not ignoring implicitly error codes, eg:
discard execShellCmd(&"mkdir {coverage} {nimcache}")
=>
import os
createDir(coverage)
createDir(nimcache)
another example:
execShellCmd(&"ls {fileinfo}") == 1 doesn't seem very portable; eg a user could have an alias for ls ; not sure how portable is return code 1, etc.
maybe os. existsFile is what you want here?
another example:
discard execShellCmd(&"rm -rf {fileinfo} {coverage} {nimcache}")
this is downright dangerous if user passes a path with a space (not that it'd be wise, but still), eg: --report_path="tests foobar"
[x] etc (likewise with other usges of execShellCmd)
The fmt"{expr}" syntax is more aesthetically pleasing, but it hides a small gotcha. The string is a generalized raw string literal. This has some surprising effects:
[x] usually, prefer const, then let, then var; eg:
var fileinfo = "test.info"
=>
const fileinfo = "test.info"
discard execShellCmd(&"mkdir {coverage} {nimcache}") => import os createDir(coverage) createDir(nimcache)
another example:
execShellCmd(&"ls {fileinfo}") == 1
doesn't seem very portable; eg a user could have an alias forls
; not sure how portable is return code 1, etc. maybeos. existsFile
is what you want here?another example:
discard execShellCmd(&"rm -rf {fileinfo} {coverage} {nimcache}")
this is downright dangerous if user passes a path with a space (not that it'd be wise, but still), eg:--report_path="tests foobar"
[x] etc (likewise with other usges of execShellCmd)
[x] from the docs,
fmt
is preferred to&
(being more obvious etc) unless in special cases see https://nim-lang.github.io/Nim/strformat.html#%26.m%2Cstring :[x] usually, prefer const, then let, then var; eg: var fileinfo = "test.info" => const fileinfo = "test.info"
[x] keep it DRY: instead of:
use:
etc
[x] IIRC
discardable
is not needed when you return void ; and for void return, we don't need to specify return type:[x] quit is evil; use doAssert ; and you can simplify:
=>