-
Notifications
You must be signed in to change notification settings - Fork 11
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
Update GitHub Action workflows to use aepsdk-commons #171
Conversation
.github/workflows/maven-release.yml
Outdated
@@ -15,62 +15,29 @@ on: | |||
workflow_dispatch: | |||
inputs: | |||
tag: | |||
description: 'tag/version' | |||
description: 'The existing tag (version) to be released (ex: 5.0.1).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tag will be created with the workflow, so "The existing tag ..." might sound confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The release workflow is currently using the --verify-tag
flag which:
Abort in case the git tag doesn't already exist in the remote repository
docs: https://cli.github.com/manual/gh_release_create
This is true for both iOS and Android release workflows - I was thinking it could be beneficial to make creating the release tag explicit, in order to avoid accidentally publishing unintended changes/branch states. What do you think?
See example release failure due to missing tag + this flag here: https://github.com/timkimadobe/aepsdk-edge-android/actions/runs/11507608386/job/32034124666
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression create GH release creates the tag and release. Isn't that the case?
.github/workflows/maven-release.yml
Outdated
default: true | ||
|
||
edge-identity-dependency: | ||
description: 'The AEPEdgeIdentity dependency version in gradle.properties to be validated (ex: 3.0.1).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Here we use ex: 3.0.1 the L18 tag has ex: 5.0.1
let's keep example versions also consistent and probably to latest major version i.e. 3.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see 3.0.1 in rest of the PR so let's use 3.0.1 everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually what do you think about using a generic 1.2.3
everywhere instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works
Add code documentation to explicitly mention tags must match
.github/workflows/update-version.yml
Outdated
@@ -1,48 +1,56 @@ | |||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the copyright header for this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Added back
.github/workflows/update-version.yml
Outdated
|
||
core-dependency: | ||
description: '[Optional] Update Core dependency in pom.xml. Example: 3.0.0' | ||
description: 'If a version is provided, update AEPCore dependency in gradle.properties (ex: 1.2.3).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify that this is a dependency in the pom.xml file, so users know this is a published dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Added the additional context to the description as suggested
|
||
identity-dependency: | ||
description: '[Optional] Update Edge Identity dependency in pom.xml. Example: 3.0.0' | ||
edge-identity-dependency: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify that this is a dependency in the pom.xm file, so users know this is a publish dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
.github/workflows/update-version.yml
Outdated
update-version: | ||
runs-on: ubuntu-latest | ||
edge-consent-dependency: | ||
description: 'If a version is provided, update AEPEdgeConsent dependency in gradle.properties (ex: 1.2.3).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify this is a testing only dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
.github/workflows/update-version.yml
Outdated
- name: Update Edge | ||
run: (./scripts/version.sh -u -v ${{ github.event.inputs.version }} -d "Core ${{ github.event.inputs.core-dependency }}, EdgeIdentity ${{ github.event.inputs.identity-dependency }}") | ||
testutils-dependency: | ||
description: 'If a version is provided, update AEPTestUtils dependency in gradle.properties (ex: 1.2.3).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify this is a testing only dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @kevinlind! Updated based on feedback
.github/workflows/update-version.yml
Outdated
@@ -1,48 +1,56 @@ | |||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Added back
.github/workflows/update-version.yml
Outdated
|
||
core-dependency: | ||
description: '[Optional] Update Core dependency in pom.xml. Example: 3.0.0' | ||
description: 'If a version is provided, update AEPCore dependency in gradle.properties (ex: 1.2.3).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Added the additional context to the description as suggested
.github/workflows/update-version.yml
Outdated
update-version: | ||
runs-on: ubuntu-latest | ||
edge-consent-dependency: | ||
description: 'If a version is provided, update AEPEdgeConsent dependency in gradle.properties (ex: 1.2.3).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
.github/workflows/update-version.yml
Outdated
- name: Update Edge | ||
run: (./scripts/version.sh -u -v ${{ github.event.inputs.version }} -d "Core ${{ github.event.inputs.core-dependency }}, EdgeIdentity ${{ github.event.inputs.identity-dependency }}") | ||
testutils-dependency: | ||
description: 'If a version is provided, update AEPTestUtils dependency in gradle.properties (ex: 1.2.3).' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
|
||
identity-dependency: | ||
description: '[Optional] Update Edge Identity dependency in pom.xml. Example: 3.0.0' | ||
edge-identity-dependency: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Final checks using production tag ( Update versions (with dependencies): timkimadobe/aepsdk-edge-ios#48
|
Description
This PR migrates the local GitHub Action workflows to use the reusable workflows available in aepsdk-commons. At a high level:
gha-android-1.0.0
), which the workflows use for controlling which version of the workflow is usedupdate-version.yml
maven-release.yml
Note
Due to current GitHub Actions YAML limitations, YAML anchors are not available, meaning that the workflow tag and input value must be maintained simultaneously. However, they are planning on adding this in 2025.
Source: actions/runner#1182
Example job runs
Note
These runs were performed in my fork of the repo, also referencing the workflows in my fork of aepsdk-commons (to avoid cross-repo checkout token permission requirements)
Update versions (no dependencies):
Update versions (dependencies AND targeting different branch
dev
- demonstrating you can update versions in any branch):Publish release (failed because no secrets for publishing available in fork):
Publish snapshot (failed because no secrets for publishing available in fork):
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: