move the BuildInitialize hook to after determine build project#2265
move the BuildInitialize hook to after determine build project#2265freddydk wants to merge 4 commits into
Conversation
|
@freddydk please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Moves the BuildInitialize hook to run after the “Determine whether to build project” step and supplies additional context to the hook via parametersJson in both template workflows.
Changes:
- Reordered the BuildInitialize hook step to run after
DetermineBuildProject - Added
parametersJsonto passbuildModeandbuildItinto the hook
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Templates/Per Tenant Extension/.github/workflows/_BuildALGoProject.yaml | Reorders BuildInitialize hook and passes build context parameters |
| Templates/AppSource App/.github/workflows/_BuildALGoProject.yaml | Same workflow change as above for the AppSource template |
| parametersJson: | | ||
| { | ||
| "buildMode": "${{ inputs.buildMode }}", | ||
| "buildIt": ${{ steps.DetermineBuildProject.outputs.BuildIt == 'True' }} |
There was a problem hiding this comment.
Rather than passing BuildIt as a parameter, wouldn't it make more sense to just skip the hook altogether if BuildIt is false? Not sure what a hook would do in a job that doesnt build anyway.
What do you think?
There was a problem hiding this comment.
Well it actually will invoke the _BuildALGoProject (to copy the files), but it would be OK for my scenario.
I also added a Cleanup hook - with these two hooks, I can integrate Fkh to AL-Go and not need anything but that.
Obviously, I would copy some overrides to the .AL-Go folder during build, but I could modify those when you guys modify the build process and keep all changes centralized.
This is how my BuildInitialize and BuildCleanup looks:
Param(
[Hashtable] $parameters
)
# AL-Go Hook
$buildIt = $parameters.buildIt
if (-not $buildIt) {
Write-Host "BuildIt is false, no need to copy fkh scripts"
return
}
$ENV:FKH_BACKEND_URL = "https://fkh-freddydk-backend.azurewebsites.net/api"
$ENV:FKH_TIMEZONE = "Europe/Copenhagen"
Write-Host "Install Freddy's Kubernetes Helper & Business Central Development Tools"
dotnet tool install -g fkh --prerelease
dotnet tool install -g Microsoft.Dynamics.BusinessCentral.Development.Tools
fkh applyALGoOverrides --output $PSScriptRootand Cleanup:
Param(
[Hashtable] $parameters
)
# AL-Go Hook
$project = $parameters.project
$buildMode = $parameters.buildMode
$buildIt = $parameters.buildIt
if (-not $buildIt) {
Write-Host "BuildIt is false, no need to remove container"
return
}
$ENV:FKH_BACKEND_URL = "https://fkh-freddydk-backend.azurewebsites.net/api"
$ENV:FKH_TIMEZONE = "Europe/Copenhagen"
$parts = "$ENV:GITHUB_REPOSITORY". Split('/')
$containerName = "$($parts[0])-$($parts[1])-$($project)-$($buildMode)-$($ENV:GITHUB_RUN_ID)".ToLower() -replace "[^a-z0-9\-]"
fkh RemoveContainer --name $containerName --useOIDCI can easily modify the PR to skip the hooks if buildIt is false - I am not going to use that.
| hookName: BuildInitialize | ||
| parametersJson: | | ||
| { | ||
| "buildMode": "${{ inputs.buildMode }}", |
There was a problem hiding this comment.
I'm wondering if buildmode should just be a standard parameter like project is
There was a problem hiding this comment.
Depends on whether hooks will be available in non-build workflows as well, but then project maybe shouldn't be a standard parameter?
Let me know what you conclude?
❔What, Why & How
I would very much like to use the new BuildInitialize hook with my Fkh integration to AL-Go, but I need to know whether the project is going to be build or not:-)