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

[steps] Add better workflow support for if condition #497

Merged
merged 3 commits into from
Jan 17, 2025

Conversation

sjchmiela
Copy link
Contributor

@sjchmiela sjchmiela commented Jan 16, 2025

Why

We want to support if conditions on individual steps in workflows.

How

  • adjusted expected template literals for ifCondition
  • adjusted interpolation context used for ifCondition
  • (somewhat unrelated) added descriptions to StepZ Zod schema so we can replace www copies with using upstream

Test Plan

Added some tests.

Copy link

linear bot commented Jan 16, 2025

@sjchmiela sjchmiela marked this pull request as ready for review January 16, 2025 18:18
@sjchmiela sjchmiela requested a review from szdziedzic as a code owner January 16, 2025 18:18
@@ -308,17 +308,14 @@ export class BuildStep extends BuildStepOutputAccessor {

let ifCondition = this.ifCondition;

if (ifCondition.startsWith('${') && ifCondition.endsWith('}')) {
if (ifCondition.startsWith('${{') && ifCondition.endsWith('}}')) {
Copy link
Member

Choose a reason for hiding this comment

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

do we allow for ${{ sth }} pattern in other places like inside of run? If we don't it's strange to support it only for if

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do!

@@ -331,6 +328,7 @@ export class BuildStep extends BuildStepOutputAccessor {
runtimePlatform: this.ctx.global.runtimePlatform,
...this.ctx.global.staticContext,
},
...this.getInterpolationContext(),
Copy link
Member

Choose a reason for hiding this comment

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

this includes deleted things, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea

@sjchmiela sjchmiela merged commit 0d2aa5c into main Jan 17, 2025
5 checks passed
@sjchmiela sjchmiela deleted the stanley/eng-14733-add-support-for-if-in-steps branch January 17, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants