chore: prefer method of the extension over system to get OS by benoitf · Pull Request #9612 · podman-desktop/podman-desktop (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation3 Commits1 Checks13 Files changed

Conversation

This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

benoitf

What does this PR do?

change the if condition (easier to mock)

Screenshot / video of UI

What issues does this PR fix or reference?

related to #9242

How to test this PR?

@benoitf

Signed-off-by: Florent Benoit fbenoit@redhat.com

deboer-tim

Choose a reason for hiding this comment

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

question: The same function still has a call to isMac(), why one and not the other? This file already has a mix of both too, I'm worried it will be confusing for future maintainer to understand why the mix.

@benoitf

question: The same function still has a call to isMac(), why one and not the other?

sure, it does not go in some sections being tested

The mix is there for legacy reason, it's taking time to rollout the usage of some new API.
some repositories have created issues for that podman-desktop/extension-bootc#881

The reason here is to get an easier mocking

@benoitf

deboer-tim