Skip to content

Fix prototype_build_details_comment action when used outside FAD context #642

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

Merged
merged 4 commits into from
Mar 21, 2025

Conversation

AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Mar 20, 2025

What does it do?

While working on updating Tumblr-Android to use the latest release-toolkit version 13.0.0 (internal ref: Tumblr/android#24388), I noticed that the PR comment for the Prototype Build (which uses direct download from S3 and does not use Firebase App Distribution) contained extra rows.

image

Notice the Build Number and Version rows in the table. The Application ID is not empty even if this Prototype Build didn't use FAD, because we happen to provide the value for this metadata explicitly though.

This PR used TDD to fix the issue:

  • Removes some duplicate tests about app icons
  • Add test to ensure we don't include FAD-related metadata if we're not using Firebase App Distribution, to make those test fail and detect the issue
  • Fix the implementation to make the tests go green and not includ FAD rows if not using FAD

Checklist before requesting a review

  • Run bundle exec rubocop to test for code style violations and recommendations.
  • Add Unit Tests (aka specs/*_spec.rb) if applicable.
  • Run bundle exec rspec to run the whole test suite and ensure all your tests pass.
  • Make sure you added an entry in the CHANGELOG.md file to describe your changes under the appropriate existing ### subsection of the existing ## Trunk section.
  • If applicable, add an entry in the MIGRATION.md file to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.

@AliSoftware AliSoftware self-assigned this Mar 20, 2025
@AliSoftware AliSoftware added the bug Something isn't working label Mar 20, 2025
@AliSoftware AliSoftware requested review from iangmaia and a team March 20, 2025 20:12
metadata['Build Number'] ||= "<code>#{release_info&.build_version}</code>"
metadata['Version'] ||= "<code>#{release_info&.display_version}</code>"
metadata[release_info&.os == 'ios' ? 'Bundle ID' : 'Application ID'] ||= "<code>#{release_info&.bundle_id}</code>"
unless release_info.nil?
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@iangmaia iangmaia left a comment

Choose a reason for hiding this comment

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

Nice catch, it's easy to ignore the details in this table.

👍

@AliSoftware AliSoftware merged commit cf9a44a into trunk Mar 21, 2025
8 checks passed
@AliSoftware AliSoftware deleted the fix/prototype_build_details_comment-action branch March 21, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants