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

Make sure builder image is pulled by default #218

Merged
merged 4 commits into from
Aug 14, 2024
Merged

Make sure builder image is pulled by default #218

merged 4 commits into from
Aug 14, 2024

Conversation

agners
Copy link
Member

@agners agners commented Aug 14, 2024

Correctly pull the image by default.

The if expression was really wrong. This hasn't been noticed in #214 since the comparison with true == "true" indeed lead to the step not being executed. The GitHub Action syntax doesn't support boolean inputs. Make this explicit using strings.

Copy link
Member

@sairon sairon left a comment

Choose a reason for hiding this comment

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

LGTM, although it was fundamentally wrong (because of the inverted condition), it should also be a string.

Avoid double negation which is error prone.
@agners agners requested a review from sairon August 14, 2024 09:55
Copy link
Member

@sairon sairon left a comment

Choose a reason for hiding this comment

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

Logic seems fine, description need to be changed though.

action.yml Outdated Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft August 14, 2024 09:58
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
@agners agners requested a review from sairon August 14, 2024 10:15
@agners agners marked this pull request as ready for review August 14, 2024 10:15
@agners agners merged commit 34ed6fa into master Aug 14, 2024
10 checks passed
@agners agners deleted the fix-no-pull branch August 14, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants