Skip to content
This repository was archived by the owner on Aug 6, 2025. It is now read-only.

Conversation

blyme
Copy link
Member

@blyme blyme commented Jun 3, 2022

No description provided.

@blyme blyme requested a review from a team June 3, 2022 13:51
Copy link
Contributor

@danquah danquah left a comment

Choose a reason for hiding this comment

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

I just verified that the workflow did not push and it seems to work!

@blyme blyme merged commit c0c4150 into main Jun 3, 2022
@blyme blyme deleted the push-on-default-branch branch June 3, 2022 13:58
@xendk
Copy link
Member

xendk commented Jun 7, 2022

Question: Didn't the if on the workflow work?

I think it's unfortunate that the workflow is named "Docker build and push" when in fact it's not pushing most of the time.

@danquah
Copy link
Contributor

danquah commented Jun 7, 2022

Question: Didn't the if on the workflow work?

No

I think it's unfortunate that the workflow is named "Docker build and push" when in fact it's not pushing most of the time.

You have a point there, I would not argue against changing it :)

@xendk
Copy link
Member

xendk commented Jun 7, 2022

No

Interesting... It shouldn't, obviously. Maybe it choked on the syntax?

When you use expressions in an if conditional, you may omit the expression syntax (${{ }}) because GitHub automatically evaluates the if conditional as an expression. For more information, see "Expressions."

You have a point there, I would not argue against changing it :)

Would be nicest with separate "Build" and "Push" jobs, but as they're isolated that wont work. Best idea I can come up with is two virtually identical workflows with different names... Unless expression syntax works in name?

@danquah
Copy link
Contributor

danquah commented Jun 7, 2022

Unless expression syntax works in name?

It does in azure DevOps pipelines - and the two gets more and more similar as time progresses so it may very well work.

@danquah
Copy link
Contributor

danquah commented Jun 7, 2022

But - we could also just name it "Build. Publish on merge" or something like that. I do prefer not having two separate instances of the action that we have to keep in sync.

@xendk
Copy link
Member

xendk commented Jun 7, 2022

We can... But the expression hack is cooler.

@xendk
Copy link
Member

xendk commented Jun 7, 2022

(and have the advantage of telling when it pushed)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants