Skip to content

distribution: filterManifests split complicated condition#46415

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:distribution_simplify_filterManifests
Sep 7, 2023
Merged

distribution: filterManifests split complicated condition#46415
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:distribution_simplify_filterManifests

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

Break up the complicated "if" condition into multiple checks.

- A picture of a cute animal (not mandatory but encouraged)

Break up the complicated "if" condition into multiple checks.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
// TODO(thaJeztah): should we also take the user-provided platform into account (if any)?
if strings.EqualFold("windows", manifestDescriptor.Platform.OS) {
if err := checkImageCompatibility("windows", manifestDescriptor.Platform.OSVersion); err != nil {
skip()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the old code we wouldn't have this debug log here but I guess it's fine

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, yes, I probably should've mentioned that. I was writing the code, and thought having a log for this case would be useful as well (with all the windows-matching already being quite "hairy").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/distribution Image Distribution kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants