Closed TimEvens closed 8 months ago
Also, if we do add a min cwin option, we should add a test to exercise that option.
Sorry, this PR was supposed to be on the fork and not the parent.
Regarding CWIN MIN. I'll look at doing a formal PR for CWIN MIN, but it might not be needed with recent changes.
@TimEvens I looked at this PR. It seems that it has three or four unrelated changes:
If we want to go forward, those should be broken up in separate PRs.
I would not mind taking in the "min_cwin" option, in a separate PR by itself. It does seem a bit like a footgun, but if that's needed in some deployment scenarios, so be it. There is a related issue: there is code in config.c to read parameters from a configuration file, but this is not deployed in the demo app. Maybe we should think about that.
On the CMake options, it might be better to open an issue first and discuss what exactly we want.
The doc changes should be simple PR, focused on the document issues.