ITADN

refactor: remove uses of `Option` in `new_with_config()`

#978Pull Requestpickx 创建于 2025-06-14
P
pickxcommented
so this PR is rather massive in terms of LoC, and I hope this is something you even want. if not, that's totally fine and feel free to reject this. while working on related features, I noticed the following: * several structs have a `new_with_config()` function which takes `Option<&config::Config>`. these functions then proceed to try and read the config to get the values out. * this is done via an assortment of `get_*`-or-use-default functions. these all need to handle the case where the config itself is `None`. what's more, if the config is indeed `None`, the default is chosen for all of them. * unit tests all go through this mechanism where the same code paths are used for both "config available" and "config unavailable". in particular, out of 272 tests that go through `module_test()`, 245 pass `None` as `config` which (imho) is just noise. thus, this PR 1) separates these constructors to either "config is available" `new_with_config()` which takes `&Config` instead of `Option<&Config>`, or "config is unavailable, so use defaults for everything"... in which case I just impl `Default::default()`. 2) changes the `get_` functions to also take a `&Config instead of `Option<&Config>`, which simplifies the logic 3) basic refactors of some of the logic in the areas I've touched. 4) removes some now-unnecessary testing logic which creates dummy data 5) separates `module_test()` to "with config" and "without config" versions in the "default" implementations, we can also do something that reuses the default values between the "load from config" and "load defaults" constructors, if you'd like.
合并状态:未合并 0 条评论