Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean cli internal structures #500

Merged
merged 4 commits into from
Mar 14, 2025
Merged

Clean cli internal structures #500

merged 4 commits into from
Mar 14, 2025

Conversation

amorenoz
Copy link
Contributor

This is the natural follow up of #464 , and as discussed in #493 (comment)

This PR merges the two-struct model into a single RetisCli struct and moves some top-level options down the relevant sub-commands.

@amorenoz amorenoz added the run-functional-tests Request functional tests to be run by CI label Feb 28, 2025
Copy link
Contributor

@atenart atenart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up on this, nice cleanup!

@atenart atenart added this to the v1.6 milestone Mar 3, 2025
Merge both structs in one keeping the double parsing because of logging.

Signed-off-by: Adrian Moreno <[email protected]>
Early initializing of the inspector based on kconf option is only used
in "collect" and "inspect". Move the option there (no need to keep it
global), and remove the ugly hack in main.rs.

In order to keep `retis_in_container.sh` working, remove the use of
RETIS_KCONF and instead map all well known config paths and suggest
users to use the `--kconf` option as failback.

Signed-off-by: Adrian Moreno <[email protected]>
Move the fixup to in between cli parsing rounds. This is when we know
the loggig level and we know if we have to redirect logging to stdout
when pager is configured.

Signed-off-by: Adrian Moreno <[email protected]>
Copy link
Contributor

@atenart atenart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice cleanup, thanks for following-up on this!

@atenart atenart merged commit 8baebeb into retis-org:main Mar 14, 2025
14 of 15 checks passed
@atenart atenart mentioned this pull request Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-functional-tests Request functional tests to be run by CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants