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

Add New CI Pipeline for Latest WindowsAppSDK #36282

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

Conversation

shuaiyuanxx
Copy link
Contributor

Summary

The old PR cannot be reopened, so I create a new PR

This PR introduces the following changes to the CI pipeline and version management:

Pipeline Enhancements:

  • Added a new script UpdateVersions.ps1 to automate the update of Microsoft.WindowsAppSDK versions across various project files.
  • Introduced a new pipeline configuration ci-using-the-latest-winappsdk.yml to build using the latest Microsoft.WindowsAppSDK.
  • Updated existing pipeline configurations to support the new useLatestWinAppSDK parameter.

Pipeline Configuration Updates:

  • Updated job-build-project.yml to handle the useLatestWinAppSDK parameter and adjust the RestoreAdditionalProjectSourcesArg accordingly.
  • Added a new template steps-update-winappsdk-and-restore-nuget.yml for updating and restoring NuGet packages with the latest Microsoft.WindowsAppSDK.
  • Added WinAPPSDK version selection, the pipeline can be manually triggered to use the specified version.

PR Checklist

  • Closes: #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Changes

New Files

  • .pipelines/UpdateVersions.ps1: Script to update Microsoft.WindowsAppSDK versions.
  • .pipelines/v2/ci-using-the-latest-winappsdk.yml: New pipeline configuration for using the latest Microsoft.WindowsAppSDK.
  • .pipelines/v2/templates/steps-update-winappsdk-and-restore-nuget.yml: Template for updating and restoring NuGet packages.

Modified Files

  • .pipelines/v2/templates/job-build-project.yml: Updated to handle useLatestWinAppSDK parameter and adjust MSBuild arguments.
  • .pipelines/v2/templates/pipeline-ci-build.yml: Added useLatestWinAppSDK parameter.

Validation Steps Performed

  • Verified that the new .pipelines/UpdateVersions.ps1 script correctly updates the Microsoft.WindowsAppSDK versions in the relevant project files.
  • Ensured that the new pipeline configuration builds successfully with the latest Microsoft.WindowsAppSDK.
  • Tested the conditional execution of verifyNoticeMdAgainstNugetPackages.ps1 based on the useLatestWinAppSDK parameter.

shuaiyuanxx and others added 13 commits December 5, 2024 13:17
Signed-off-by: Shawn Yuan <[email protected]>
Signed-off-by: Shawn Yuan <[email protected]>
Signed-off-by: Shawn Yuan <[email protected]>
Signed-off-by: Shawn Yuan <[email protected]>
Pipeline can now specify the version of winappsdk to be used.
change to daily midnight build
@crutkas crutkas added the Needs-Review This Pull Request awaits the review of a maintainer. label Dec 10, 2024
@crutkas
Copy link
Member

crutkas commented Dec 11, 2024

FYI, you can mark it as draft until you're ready.
image

@shuaiyuanxx
Copy link
Contributor Author

FYI, you can mark it as draft until you're ready. image

Got it, it's ready for review now

@jaimecbernardo jaimecbernardo added the Don't merge - Hold for release Hold off on merging this PR, even if it's approved. label Dec 11, 2024
@jaimecbernardo
Copy link
Collaborator

Hi, we've added the "Don't Merge" tag to the PR, since we're trying to keep the repo stable for the release and possible hotfix if necessary. Please don't merge the PR while the tag is still in here. This allows people to review the PR and approve with less fear that it'll get merged 😄

@shuaiyuanxx shuaiyuanxx requested a review from a team as a code owner December 16, 2024 02:17
Copy link

@yeelam-gordon yeelam-gordon left a comment

Choose a reason for hiding this comment

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

Thanks for the change, overall looks good, have minor comments.

@@ -233,10 +254,16 @@ jobs:
inputs:
solution: '**\HostsUILib.csproj'
vsVersion: 17.0
${{ if eq(parameters.useVSPreview, true) }}:
msbuildArgs: /p:CIBuild=true;NoBuild=true -t:pack /bl:$(LogOutputDirectory)\build-hosts.binlog /p:NoWarn=NU5104
${{ if or(eq(parameters.useVSPreview, true), eq(parameters.useLatestWinAppSDK, true)) }}:

Choose a reason for hiding this comment

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

Oh, with this, we can make it as single line then.
Since I see there is no security concern for "making up the build parameters", as the result, we can directly like WITHOUT IF ELSE
msbuildArgs: >-
/p:CIBuild=true;NoBuild=true -t:pack
/bl:$(LogOutputDirectory)\build-hosts.binlog
$(MSBuildArgs.UseVSPreview)
$(RestoreAdditionalProjectSourcesArg)

[string]$versionNumber = "1.6",

#[Parameter(Mandatory=$False,Position=2)]
[string]$useExperimentalVersion = "false"

Choose a reason for hiding this comment

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

Any reason to be string instead of boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the pipeline calls the PS1 file, the passed parameters are converted to string type.

Copy link
Member

Choose a reason for hiding this comment

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

That is unexpected.

You can pass any type of PowerShell object to a script in a pipeline.

pwsh: |-
  & ./Path/To/Script.ps1 -BooleanArgunent $true

If you need to convert an azure DevOps yaml boolean into a PowerShell boolean, you can use...

$${{parameters.azureDevopsYamlBoolean}}

This will evaluate to $true or $false (the first $ is for PowerShell, and the second one is for Azure DevOps!)

# print nuget.config after modification
$xml.OuterXml
# Save the modified nuget.config file
$xml.Save($filePath)

Choose a reason for hiding this comment

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

For these big chunk of code, would you mind to break it down into smaller functions?
As the result, the main one will looks cleaner

Choose a reason for hiding this comment

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

e.g.
function Update-PackageConfig {
param (
[string]$filePath,
[string]$oldVersionString,
[string]$newVersionString
)
# Function implementation
}

.pipelines/UpdateVersions.ps1 Show resolved Hide resolved
Signed-off-by: Shawn Yuan <[email protected]>
Signed-off-by: Shawn Yuan <[email protected]>
@crutkas crutkas added Good to merge after release and removed Needs-Review This Pull Request awaits the review of a maintainer. labels Dec 18, 2024
@jaimecbernardo jaimecbernardo removed the Don't merge - Hold for release Hold off on merging this PR, even if it's approved. label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

6 participants