KomuSolutions/WANessa
Archived
4
0
Fork 0
Code Issues 26 Pull Requests 1 Actions Projects 1 Releases Activity

Logging #26

Open
leon wants to merge 33 commits from Logging into main
pull from: Logging
merge into: KomuSolutions:main
Conversation 11 Commits 33 Files Changed 8 +15 -15
1 changed files with 15 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit d8b7616c43 - Show all commits

30
config/src/lib.rs
View File

@ -16,9 +16,9 @@ use getset::Setters;
* Represents a valid `WANessa` configuration. Intended as a read-only singleton.
* See [`DEFAULTS`]
*/
#[derive(Clone, PartialEq, Eq, Getters, Setters)]
#[derive(Clone, PartialEq, Eq, Getters, Setters, Debug)]
#[getset(get = "pub", set = "pub")]
#[allow(clippy::struct_excessive_bools)] // False positive, since it is a config struct.
#[allow(clippy::struct_excessive_bools)] // False positive, since this is a config struct.
pub struct Config {
/// See [`LogVerbosity`].<br>
leon commented 2024-02-15 18:12:46 +01:00
Outdated
Review
Copy Link

-> Maybe just remove the setters and use an initializer to ensure immutability

I considered this.
I just thought that setters are more convenient than using mem::swap or *config = Config::new(...). I also was not sure, if the config is truly read-only since we never specified if hot-reloading the config is a desired feature. Depending on how the CLI is implemented, hot-applying config-changes might also be a necessity to disable stdout logging, which would pollute the CLIs visuals.

@hendrik

> -> Maybe just remove the setters and use an initializer to ensure immutability I considered this. I just thought that setters are more convenient than using `mem::swap` or `*config = Config::new(...)`. I also was not sure, if the config is truly read-only since we never specified if hot-reloading the config is a desired feature. Depending on how the CLI is implemented, hot-applying config-changes might also be a necessity to disable stdout logging, which would pollute the CLIs visuals. @hendrik
hendrik commented 2024-02-15 19:11:07 +01:00
Outdated
Review
Copy Link

I assumed the config would be read only, because some attribute changes after initialization could easily cause unexpected problems. For example, changing the database server at runtime is not something the current code is designed for, nor would it be a sensible feature to implement. If the log-settings should be mutable after the initialization phase, we should add a mutable reference to some "LogPreferences" structure to our Config instead of generally supporting hot-reload for the whole Config

@leon

I assumed the config would be read only, because some attribute changes after initialization could easily cause unexpected problems. For example, changing the database server at runtime is not something the current code is designed for, nor would it be a sensible feature to implement. If the log-settings should be mutable after the initialization phase, we should add a mutable reference to some "LogPreferences" structure to our Config instead of generally supporting hot-reload for the whole Config @leon
leon commented 2024-02-15 19:12:33 +01:00
Outdated
Review
Copy Link

Seems sensible. It is also probably a good idea to split the config struct into multiple parts.

Seems sensible. It is also probably a good idea to split the config struct into multiple parts.
leon commented 2024-02-15 21:56:27 +01:00
Outdated
Review
Copy Link

See a787dd93e5

See a787dd93e5
/// Default: [`Warning`]
@ -92,19 +92,19 @@ impl Default for Config {
* ```rust
* # use config::Config;
* # use config::LogVerbosity::Warning;
* let DEFAULTS = Config
* {
* log_verbosity: Warning,
* log_time: false,
* log_time_format: None,
* log_location: false,
* log_stdout: true,
* log_stderr: true,
* log_path: None,
* db_addr: None,
* db_port: 6969,
* };
* # assert!(DEFAULTS == config::DEFAULTS)
* let mut defaults = Config::default();
*
* defaults.set_log_verbosity(Warning);
* defaults.set_log_time(false);
* //defaults.set_log_time_format(None);
* defaults.set_log_location(false);
* defaults.set_log_stdout(true);
* defaults.set_log_stderr(true);
* //defaults.set_log_path(None);
* //defaults.set_db_addr(None);
* defaults.set_db_port(6969);
*
* # assert!(defaults == config::DEFAULTS)
* ```
*/
pub const DEFAULTS: Config = Config {