Fix actionlint failure on generated maintenance workflow operation input
#56506
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,51 @@ import "github.com/github/gh-aw/pkg/logger" | |
|
|
||
| var maintenanceConditionsLog = logger.New("workflow:maintenance_conditions") | ||
|
|
||
| // maintenanceNoOperationValue is the sentinel value used for the workflow_dispatch | ||
| // "operation" choice input to represent "no operation selected". It replaces an | ||
| // empty string option, since actionlint rejects empty strings as workflow_dispatch | ||
| // choice options and requires the default to be one of the declared options. | ||
| // The workflow_call "operation" input remains a plain string with an empty-string | ||
| // default (not subject to the choice/options restriction), so the empty value and | ||
| // this sentinel value are treated as "no operation" throughout the generated conditions. | ||
| const maintenanceNoOperationValue = "none" | ||
|
|
||
| // buildOperationIsEmptyCondition creates a condition that is true when the | ||
| // `inputs.operation` value represents "no operation selected", i.e. it is | ||
| // either an empty string (workflow_call default) or the choice sentinel value | ||
| // used by the workflow_dispatch "operation" input. | ||
| func buildOperationIsEmptyCondition() ConditionNode { | ||
| return BuildOr( | ||
| BuildEquals( | ||
| BuildPropertyAccess("inputs.operation"), | ||
| BuildStringLiteral(""), | ||
| ), | ||
| BuildEquals( | ||
| BuildPropertyAccess("inputs.operation"), | ||
|
Contributor
There was a problem hiding this comment. [/tdd] The two new helpers 💡 Suggested testAdd a small table-driven test in func TestBuildOperationIsEmptyCondition(t *testing.T) {
node := buildOperationIsEmptyCondition()
got := renderCondition(node) // use whatever renderer is available
want := `inputs.operation == '' || inputs.operation == 'none'`
require.Equal(t, want, got)
}
func TestBuildOperationIsNotEmptyCondition(t *testing.T) {
node := buildOperationIsNotEmptyCondition()
got := renderCondition(node)
want := `inputs.operation != '' && inputs.operation != 'none'`
require.Equal(t, want, got)
}This gives a regression anchor directly on the helpers and documents their intended output as a specification. @copilot please address this. |
||
| BuildStringLiteral(maintenanceNoOperationValue), | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| // buildOperationIsNotEmptyCondition creates a condition that is true when the | ||
| // `inputs.operation` value represents an operation being selected, i.e. it is | ||
| // neither the empty string (workflow_call default) nor the choice sentinel | ||
| // value used by the workflow_dispatch "operation" input. Built directly from | ||
| // BuildNotEquals (rather than negating buildOperationIsEmptyCondition) so it | ||
| // renders using the same `!=` style as the other operation exclusion checks. | ||
| func buildOperationIsNotEmptyCondition() ConditionNode { | ||
| return BuildAnd( | ||
| BuildNotEquals( | ||
| BuildPropertyAccess("inputs.operation"), | ||
| BuildStringLiteral(""), | ||
| ), | ||
| BuildNotEquals( | ||
| BuildPropertyAccess("inputs.operation"), | ||
| BuildStringLiteral(maintenanceNoOperationValue), | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| // buildNotForkCondition creates a condition to check the repository is not a fork. | ||
| func buildNotForkCondition() ConditionNode { | ||
| return &NotNode{ | ||
|
|
@@ -26,10 +71,7 @@ func buildNotDispatchOrCallOrEmptyOperation() ConditionNode { | |
| BuildStringLiteral("workflow_call"), | ||
| ), | ||
| ), | ||
| buildOperationIsEmptyCondition(), | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -145,10 +187,7 @@ func buildRunOperationCondition(excludedOperations ...string) ConditionNode { | |
| BuildEventTypeEquals("workflow_dispatch"), | ||
| BuildEventTypeEquals("workflow_call"), | ||
| ), | ||
| buildOperationIsNotEmptyCondition(), | ||
| ) | ||
|
|
||
| // Exclude each dedicated operation | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -352,9 +352,9 @@ on: | |
| description: 'Optional maintenance operation to run' | ||
| required: false | ||
| type: choice | ||
|
Contributor
There was a problem hiding this comment. [/diagnosing-bugs] String concatenation here is inconsistent with 💡 Suggested changeAdopt the same return fmt.Sprintf(` workflow_dispatch:
inputs:
operation:
...
default: '%[1]s'
options:
- '%[1]s'
`, maintenanceNoOperationValue)This keeps the two YAML-building functions consistent and makes the substitution explicit. @copilot please address this. |
||
| default: '` + maintenanceNoOperationValue + `' | ||
| options: | ||
| - '` + maintenanceNoOperationValue + `' | ||
| - 'safe_outputs' | ||
| - 'create_labels' | ||
| - 'activity_report' | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/workflow/maintenance_conditions.go:16-50: yagni: two tiny helpers for one extra / branch. Inline the comparisons directly at the call sites.