Add package workflow #491

mkellerman · 2023-12-19T18:52:45Z

Description

Motivation and Context

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My code follows the code style of this project.
I have added Pester Tests that describe what my changes should do.
I have updated the documentation accordingly.

.github/workflows/build_and_test.yml

lipkau · 2023-12-19T21:26:26Z

.github/workflows/build_and_test.yml

@@ -204,3 +204,37 @@ jobs:
          name: ${{ runner.os }}-Unit-Tests
          path: Test*.xml
    if: ${{ always() }}
+
+  package:
+    name: Package


I don't think Package is needed in the "CI" part of the pipeline.
We don't need to generate the package every time someone pushes a new commit to a PR.

And Package is also defined (currently double) in the package_deploy.yml -- where I think it actually belongs in.

Correct. I was originally thinking of having every PR generate a package as well, but this is overkill.

Will cleanup before converting this Draft PR to a real PR.

.github/workflows/package_deploy.yml

lipkau · 2023-12-19T21:29:57Z

.github/workflows/package_deploy.yml

+  push:
+    branches:
+      - feature/package-artifact
+  workflow_dispatch:


this is cool!
but does it need the :?
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_dispatch

I've always had it, I guess it's a force of habit. It's also how Copilot suggested it. 🤷‍♂️

lipkau · 2023-12-19T21:30:59Z

.github/workflows/package_deploy.yml

+        shell: pwsh
+        run: |
+          ./Tools/setup.ps1
+          Invoke-Build -Task ShowInfo


consider removing this, if we don't need it.
I added this back in the day because the information was important to debug failed builds

I find it useful and doesn't impact negatively the process. But your call.

lipkau · 2023-12-19T21:33:03Z

.github/workflows/package_deploy.yml

+      - name: Package
+        shell: pwsh
+        run: |
+          Invoke-Build -Task Package


can't we inline this with the previous task?

Invoke-Build -Task Clean, Build, Package

it's not like the tasks would benefit from the split, right?

Keeping the steps separate means it's easy to see quickly where it failed and why.

Not benefit to merging them together IMO. But you're call.

lipkau · 2023-12-19T21:35:32Z

.github/workflows/package_deploy.yml

+        run: |
+          Invoke-Build -Task Package
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v3


Can we add if-no-files-found: error (default is warning")?
https://github.com/actions/upload-artifact#customization-if-no-files-are-found

The package step is 'new', it's so we can generate the zip file for the module. I'm not sure yet if it's even worth keeping once we add the release mechanism to GitHub. But will make the changes. Good for it to fail if not found.

lipkau · 2023-12-19T21:39:35Z

JiraPS.build.ps1

+
+    $Prerelease = ''
+    if ("$env:BHBranchName" -notin @('master','main')) {
+        $Prerelease = "$env:BHBranchName".ToLower() -replace '[^a-zA-Z0-9]'


I would like more documentation on this.
I would expect $Prerelease to be empty or -eq "alpha". But I don't know how you would name branches to get to that value

https://learn.microsoft.com/en-us/powershell/gallery/concepts/module-prerelease-support?view=powershellget-3.x#identifying-a-module-version-as-a-prerelease

Prerelease string may only be specified when the ModuleVersion is 3 segments for Major.Minor.Build. This aligns with SemVer v1.0.0.

A hyphen is the delimiter between the Build number and the Prerelease string. A hyphen may be included in the Prerelease string as the first character, only.

The Prerelease string may contain only ASCII alphanumerics [0-9A-Za-z-]. It is a best practice to begin the Prerelease string with an alpha character, as it will be easier to identify that this is a prerelease version when scanning a list of packages.

Only SemVer v1.0.0 prerelease strings are supported at this time. Prerelease string must not contain either period or + [.+], which are allowed in SemVer 2.0.

Examples of supported Prerelease string are: -alpha, -alpha1, -BETA, -update20171020

lipkau · 2023-12-19T21:40:39Z

JiraPS.build.ps1

+
+    $Prerelease = ''
+    if ("$env:BHBranchName" -notin @('master','main')) {
+        $Prerelease = "$env:BHBranchName".ToLower() -replace '[^a-zA-Z0-9]'


the quotations around the $env: are not needed

lipkau · 2023-12-19T21:42:33Z

JiraPS.build.ps1

@@ -194,6 +194,13 @@ task UpdateManifest GetNextVersion, {
    if ($ModuleAlias) {
        BuildHelpers\Update-Metadata -Path "$env:BHBuildOutput/$env:BHProjectName/$env:BHProjectName.psd1" -PropertyName AliasesToExport -Value @($ModuleAlias.Name)
    }
+
+    $Prerelease = ''
+    if ("$env:BHBranchName" -notin @('master','main')) {


if you are checking the branch for both master and main -- which I am not opposed to -- should we use both values in the workflows too? (.github/workflows/build_and_test.yml#6)

lipkau · 2023-12-19T21:48:09Z

.github/workflows/package_deploy.yml

+      - name: Upload Artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: JiraPS.zip


https://github.com/actions/upload-artifact#zip-archives

The uploading of an artifact creates a .zip for the upload.
So our build artefact is JiraPS.zip with a content of JiraPS.zip.

maybe we should remove the zipping in Invoke-Build -Task Package

lipkau · 2023-12-19T21:50:51Z

.github/workflows/package_deploy.yml

+      - name: Build
+        shell: pwsh
+        run: |
+          Invoke-Build -Task Clean, Build


do we need this?
can't we download the artifact from Upload Artifact from .github/workflows/build_and_test.yml?

lipkau · 2023-12-19T21:57:04Z

By looking into the job that ran for this PR it became quite obvious for me, that the "Deploy Module" should only consist of the step:

Set up job
actions/checkout (we need the deployment code after all)
(missing) Download Artifact
caching (I think)
Setup
Deploy
cleanup which is not managed by us

The current list of jobs is quite large, adds steps which could fail or need debugging and it's counter intuitive (aka: it reads poorly to have a "Build" step in the "Deploy Module")

JiraPS.build.ps1

mkellerman added 6 commits December 19, 2023 10:51

Add package workflow

c673a18

Add package and deploy workflow

1feb127

Add Prerelease support to UpdateManifest task

1239c96

Add Prerelease field to JiraPS.psd1

dbeb881

Remove unnecessary steps in package_deploy.yml

417cf59

Update workflow to deploy module

ebe77cb

lipkau reviewed Dec 19, 2023

View reviewed changes

.github/workflows/build_and_test.yml Outdated Show resolved Hide resolved

lipkau reviewed Dec 19, 2023

View reviewed changes

.github/workflows/package_deploy.yml Outdated Show resolved Hide resolved

lipkau reviewed Dec 19, 2023

View reviewed changes

lipkau force-pushed the feature/package-artifact branch 2 times, most recently from 6a5ffc3 to 0a21726 Compare December 20, 2023 14:37

POC: Refactored CI pipeline

93b30b8

lipkau force-pushed the feature/package-artifact branch from 0a21726 to 93b30b8 Compare December 20, 2023 15:01

lipkau reviewed Dec 20, 2023

View reviewed changes

JiraPS.build.ps1 Show resolved Hide resolved

lipkau force-pushed the feature/package-artifact branch from 470222c to d747e84 Compare December 21, 2023 10:12

lipkau added 2 commits December 21, 2023 12:20

testing workflow_run for CI to trigger CD

cc09a14

testing caching

ca217bc

lipkau force-pushed the feature/package-artifact branch from d747e84 to ca217bc Compare December 21, 2023 11:20

lipkau changed the base branch from master to develop December 21, 2023 13:14

Updated pipeline, as I forgot about the develop branch

248785a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add package workflow #491

Add package workflow #491

mkellerman commented Dec 19, 2023

lipkau Dec 19, 2023

mkellerman Dec 19, 2023

lipkau Dec 19, 2023

mkellerman Dec 19, 2023

lipkau Dec 19, 2023

mkellerman Dec 19, 2023

lipkau Dec 19, 2023

mkellerman Dec 19, 2023

lipkau Dec 19, 2023

mkellerman Dec 19, 2023

lipkau Dec 19, 2023

lipkau Dec 19, 2023

lipkau Dec 19, 2023

lipkau Dec 19, 2023

lipkau Dec 19, 2023

lipkau commented Dec 19, 2023

Add package workflow #491

Are you sure you want to change the base?

Add package workflow #491

Conversation

mkellerman commented Dec 19, 2023

Description

Motivation and Context

Types of changes

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lipkau commented Dec 19, 2023