chore: warning cleanup #1055
chore: warning cleanup #1055
Conversation
codex/sales/states/filled.nim
Outdated
| @@ -10,7 +10,8 @@ import ./errored | |||
| import ./cancelled | |||
| import ./failed | |||
| import ./proving | |||
| import ./provingsimulated | |||
| import ./provingsimulated # used in tests | |||
| {.push warning[UnusedImport]:off.} | |||
There was a problem hiding this comment.
Hm... why do we have an import that's used only in tests inside of a production module? Shouldn't we keep this warning around and address that instead?
There was a problem hiding this comment.
Because we need to have some way how to bypass the "normal proving" module, as generating normal proofs in tests would take too long, and we also need a way to control how and when proving should fail. This functionality is hidden behind compile flag (see here: https://github.com/codex-storage/nim-codex/pull/1055/files/c7074e42d82fe32fee7056eab1fd0e2e8cb9e942#diff-c422f0a891c99abb9c168acd1d6d88f2bd50b0571521e10fb5668a759a9e0e0dR56) so it is not really shipped in production.
I originally tried to move the import in "local import" fashion to the when below, but that does not work in Nim, but you gave me an idea to just add when to the import then ;-)
|
|
||
| {.push warning[UnusedImport]: on.} | ||
| import std/terminal # Is not used in tests | ||
| {.pop.} |
There was a problem hiding this comment.
Ugh... OK. I guess this points out that we should have a config that's specific to tests, but OK, maybe not worth the trouble for a single module.
I was fed up with so long build logs full of warnings, so did small cleanup mainly for
UnusedImports, during which I discovered an actually bug in Nim: nim-lang/Nim#24552There are a few more areas that would be good to clean up that I listed here: #1054
Unfortunately this PR mainly helped the Test's build logs, where there were lot of unused imports. For normal compile logs the main source of the warnings is from Chronos's async-raises and deprected
cidthat I skipped for now.