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.

Sign up for GitHub

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 package workflow #491

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
Draft

Add package workflow #491

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c673a18
Add package workflow
mkellerman Dec 19, 2023
1feb127
Add package and deploy workflow
mkellerman Dec 19, 2023
1239c96
Add Prerelease support to UpdateManifest task
mkellerman Dec 19, 2023
dbeb881
Add Prerelease field to JiraPS.psd1
mkellerman Dec 19, 2023
417cf59
Remove unnecessary steps in package_deploy.yml
mkellerman Dec 19, 2023
ebe77cb
Update workflow to deploy module
mkellerman Dec 19, 2023
93b30b8
POC: Refactored CI pipeline
lipkau Dec 20, 2023
cc09a14
testing `workflow_run` for CI to trigger CD
lipkau Dec 21, 2023
ca217bc
testing caching
lipkau Dec 21, 2023
248785a
Updated pipeline, as I forgot about the `develop` branch
lipkau Dec 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .github/workflows/build_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,37 @@ jobs:
name: ${{ runner.os }}-Unit-Tests
path: Test*.xml
if: ${{ always() }}

package:
name: Package
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

needs: [build_module, test_on_windows_v5, test_on_windows_v7, test_on_ubuntu, test_on_macos, test_against_cloud]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Download
uses: actions/download-artifact@v3
with:
name: Release
path: ./Release/
- name: Setup PowerShell module cache
lipkau marked this conversation as resolved.
Show resolved Hide resolved
id: cacher
uses: actions/cache@v3
with:
path: "~/.local/share/powershell/Modules"
key: ${{ runner.os }}-PSModules
- name: Setup
shell: pwsh
run: |
./Tools/setup.ps1
Invoke-Build -Task ShowInfo
- name: Package
shell: pwsh
run: |
Invoke-Build -Task Package
- name: Upload Artifact
uses: actions/upload-artifact@v3
with:
name: JiraPS.zip
path: ./Release/JiraPS.zip
if: ${{ success() }}

46 changes: 46 additions & 0 deletions .github/workflows/package_deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: Package and Deploy

on:
push:
branches:
- feature/package-artifact
lipkau marked this conversation as resolved.
Show resolved Hide resolved
workflow_dispatch:
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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



jobs:
build_package_deploy:
name: Deploy Module
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Setup PowerShell module cache
id: cacher
uses: actions/cache@v3
with:
path: "~/.local/share/powershell/Modules"
key: ${{ runner.os }}-PSModules
- name: Setup
shell: pwsh
run: |
./Tools/setup.ps1
Invoke-Build -Task ShowInfo
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

- name: Build
shell: pwsh
run: |
Invoke-Build -Task Clean, Build
Copy link
Member

Choose a reason for hiding this comment

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

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

- name: Package
shell: pwsh
run: |
Invoke-Build -Task Package
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

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.

- name: Upload Artifact
uses: actions/upload-artifact@v3
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

with:
name: JiraPS.zip
Copy link
Member

Choose a reason for hiding this comment

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

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

path: ./Release/JiraPS.zip
- name: Deploy
shell: pwsh
run: |
# Invoke-Build -Task Deploy
Write-Warning "SKIPPING: Invoke-Build -Task Deploy"
if: ${{ success() }}

7 changes: 7 additions & 0 deletions JiraPS.build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Copy link
Member

Choose a reason for hiding this comment

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

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)

$Prerelease = "$env:BHBranchName".ToLower() -replace '[^a-zA-Z0-9]'
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

the quotations around the $env: are not needed

}
BuildHelpers\Update-Metadata -Path "$env:BHBuildOutput/$env:BHProjectName/$env:BHProjectName.psd1" -PropertyName Prerelease -Value $Prerelease

}

# Synopsis: Create a ZIP file with this build
mkellerman marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
3 changes: 3 additions & 0 deletions JiraPS/JiraPS.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@
# External dependent modules of this module
# ExternalModuleDependencies = ''

# Prerelease string of this module
Prerelease = ''

} # End of PSData hashtable

} # End of PrivateData hashtable
Expand Down
Loading