Skip to content

Conversation

@Vafilor
Copy link
Contributor

@Vafilor Vafilor commented Jan 9, 2021

What this PR does:

Which issue(s) this PR fixes:

Fixes #828

Special notes for your reviewer:

Checklist

Please check if applies

  • I have added/updated relevant unit tests
  • I have added/updated relevant documentation

Required

  • I accept to release these changes under the Apache 2.0 License

@Vafilor Vafilor added this to the v0.18.0 milestone Jan 9, 2021
// Strip out whitespace from parameter value
formattedValue := strings.Replace(*param.Value, " ", "", -1)
*param.Value = strings.Replace(formattedValue, "{{workflow.namespace}}", namespace, -1)
*param.Value = strings.Replace(*param.Value, "{{workspace.namespace}}", namespace, -1)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this use formattedValue too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rushtehrani Do you mean the 3rd line there?

*param.Value = strings.Replace(*param.Value, "{{workspace.namespace}}", namespace, -1)

If so, no. It is correct because we replace workflow.namespace above. If we set it to formattedValue we would clear out those changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah got it. Minor: might be cleaner to use ReplaceAll - it calls Replace with -1 internally.

@rushtehrani rushtehrani merged commit 302731e into onepanelio:master Jan 10, 2021
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.

Allow variable references in Workflow and Workspace parameters

2 participants