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

Run "Update AL-Go System Files" on multiple branches #1037

Open
wants to merge 86 commits into
base: main
Choose a base branch
from

Conversation

mazhelez
Copy link
Collaborator

@mazhelez mazhelez commented Apr 15, 2024

Issue: When scheduling "Update AL-Go System Files" workflow, it runs only on the default branch (GitHub Actions limitation).

This PR alters the workflow to be able to schedule "Update AL-Go System Files" to run on multiple branches.
Manually dispatching the workflow isn't changed.

New property workflowSchedule setting, includeBranches, is added to specify the branches when running the workflow on a schedule.

@mazhelez mazhelez marked this pull request as ready for review February 5, 2025 21:32
@mazhelez mazhelez mentioned this pull request Feb 6, 2025
4 tasks
@mazhelez mazhelez changed the title Schedule "Update AL-Go System Files" to run on multiple branches Run "Update AL-Go System Files" on multiple branches Feb 6, 2025
spetersenms
spetersenms previously approved these changes Feb 10, 2025
Copy link
Contributor

@freddydk freddydk left a comment

Choose a reason for hiding this comment

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

Here is a partial review, with some suggestions to think about


When run on a schedule, _Update AL-Go System Files_ only runs on the _main_ branch. By setting `includeBranches` in `workflowSchedule` setting, you can now run the workflow on a schedule on multiple branches. Read more at https://aka.ms/algosecrets#workflowSchedule.

Dispatching the workflow manually still runs the workflow only on the branch it was dispatched on.
Copy link
Contributor

Choose a reason for hiding this comment

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

The includeBranches is also available when running manually right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The input takes priority when dispatching the workflow.

So, there's includeBranches input: used when running manually.
workflowSchedule.includeBranches setting: used when running on a schedule.

I had an iteration where the input was something like "run the same way as scheduled", but I quickly realized how it can be confusing. Especially when there's also commitOptions setting.

@@ -705,6 +705,10 @@ function ReadSettings {
"messageSuffix" = ""
"pullRequestAutoMerge" = $false
"pullRequestLabels" = @()
"createPullRequest" = $true
}
"workflowSchedule" = [ordered] @{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably don't need to add it in default settings.

switch ($env:GITHUB_EVENT_NAME) {
'schedule' {
Write-Host "Event is schedule: getting branches from settings"
$branchPatterns = @($($(ConvertFrom-Json $env:settings).workflowSchedule.includeBranches))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add defensive check in case workflowSchedule or includeBranches is not defined.

@@ -63,7 +63,7 @@ $settings.Keys | ForEach-Object {
}
$outSettings += @{ "$setting" = $settingValue }
if ($getSettings -contains $setting) {
if ($settingValue -is [System.Collections.Specialized.OrderedDictionary] -or $settingValue -is [hashtable]) {
if ($settingValue -is [System.Collections.Specialized.OrderedDictionary] -or $settingValue -is [hashtable] -or $settingValue -is [array]) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not needed.

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.

3 participants