feat: Add --log-level to tedge-write and tedge-apt-plugin#3271
feat: Add --log-level to tedge-write and tedge-apt-plugin#3271Bravo555 merged 2 commits intothin-edge:mainfrom
--log-level to tedge-write and tedge-apt-plugin#3271Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files📢 Thoughts on this report? Let us know! |
170f867 to
47f3fe2
Compare
Robot Results
|
| #[command(flatten)] | ||
| log_args: LogConfigArgs, | ||
|
|
||
| /// [env: TEDGE_CONFIG_DIR, default: /etc/tedge] | ||
| #[clap( | ||
| long = "config-dir", | ||
| default_value = tedge_config::get_config_dir().into_os_string(), | ||
| hide_env_values = true, | ||
| hide_default_value = true, | ||
| )] | ||
| config_dir: Utf8PathBuf, |
There was a problem hiding this comment.
As @reubenmiller proposed in on of the earlier PRs, wouldn't it make sense to capture these two arguments into a common struct and use a flattened reference to it in all these concrete structs like MapperOpt, AgentOpt etc?
There was a problem hiding this comment.
We could put config_dir into the common struct as well, but because it's a multicall binary we'd still have to include it in Opt struct for each component and can't really put it in a single place and make all the other Opts have references to it. They have to own it directly.
There was a problem hiding this comment.
I think I may have wrongly used the word "reference" here, which caused that confusion. What I meant was to have a flattened field, something like:
#[command(flatten)]
common_args: CommonArgs,
and not really a "reference" to a single instance of that struct, which probably would unnecessarily complicated the code.
albinsuresh
left a comment
There was a problem hiding this comment.
As discussed, extracting the common args into a common struct and using it at one place to enable logging for all components would be attempted in a follow-up PR.
Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
47f3fe2 to
4742be8
Compare
--log-level to tedge-write and tedge-apt-plugin--log-level to tedge-write and tedge-apt-plugin
Proposed changes
Add
--log-levelflag totedge-writeandtedge-apt-plugin.Types of changes
Paste Link to the issue
Checklist
cargo fmtas mentioned in CODING_GUIDELINEScargo clippyas mentioned in CODING_GUIDELINESFurther comments