Add GitHub release asset upload action#743
Conversation
iangmaia
force-pushed
the
iangmaia/upload-github-release-assets
branch
from
0355e29 to
b98491b
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Fastlane action to upload assets to an existing GitHub Release (by tag), delegating the upload/replace behavior to GithubHelper, and includes RSpec coverage plus a changelog entry.
Changes:
- Introduces
upload_github_release_assetsaction to upload one or more local files as GitHub Release assets, with optional replace behavior. - Adds
GithubHelper#upload_release_assets(and#get_release) to locate a release by tag, list existing assets, optionally delete matching filenames, and upload new assets. - Adds unit tests for both the action and the helper behavior, and documents the new feature in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/upload_github_release_assets_spec.rb | Adds action-level tests for parameter forwarding and validation failures. |
| spec/github_helper_spec.rb | Adds helper-level tests for uploading, replacing, and error cases (missing release/file, duplicates with replace disabled). |
| lib/fastlane/plugin/wpmreleasetoolkit/helper/github_helper.rb | Implements release lookup and asset upload/replace logic with local file validation. |
| lib/fastlane/plugin/wpmreleasetoolkit/actions/common/upload_github_release_assets_action.rb | New Fastlane action entry point and options wiring for asset upload. |
| CHANGELOG.md | Adds an unreleased “Trunk” entry describing the new action/helper feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| asset_paths = Array(assets) | ||
| UI.user_error!('You must provide at least one release asset') if asset_paths.empty? | ||
|
|
||
| asset_paths.each do |file_path| | ||
| UI.user_error!("Can't find file #{file_path}!") unless File.file?(file_path) | ||
| end |
There was a problem hiding this comment.
Copilot makes a good point. Added a failing test via #745 to track this refinement.
There was a problem hiding this comment.
Oh it's a nice idea to add a failing test in a PR review like this 👍 thanks!
I've merged it and CI is 🟢
iangmaia
force-pushed
the
iangmaia/upload-github-release-assets
branch
from
b98491b to
228263c
Compare
These examples document the helper validation gaps from PR #743: non-string asset entries should fail through `UI.user_error!`, and duplicate local filenames should be rejected before GitHub mutations. --- Generated with the help of Codex, https://openai.com/codex Co-Authored-By: Codex GPT-5 <noreply@openai.com>
| existing_assets = client.release_assets(release.url) | ||
|
|
||
| asset_paths.each do |file_path| | ||
| file_name = File.basename(file_path) |
There was a problem hiding this comment.
This risks overriding files with the same name but from different folders (ios/app.ipa and tvos/app.ipa).
Is this a legitimate concern? If so, what's a good way to address this? Compare using the full path stripped of the machine-specific path to the project folder maybe?
I added a test for this in #745
What does it do?
Adds
upload_github_release_assetsfor uploading assets to an existing GitHub release by tag. Retries replace only matching filenames by default, preserve unrelated assets, and fail clearly for missing releases, missing files, or duplicate assets whenreplace_existing: false.Checklist before requesting a review
bundle exec rubocop --cache falseto test for code style violations and recommendations.specs/*_spec.rb) if applicable.bundle exec rspecto run the whole test suite and ensure all your tests pass. Focused specs pass; full suite is blocked locally by Git LFS and Xcodegenstringsenvironment prerequisites.CHANGELOG.mdfile to describe your changes under the appropriate existing###subsection of the existing## Trunksection.MIGRATION.mdfile to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider. Not applicable.