Apply review nitpicks from the TestFlight CI PR (#25674) (#25709)

* Trim transient RFC context from TestFlight CI comments

The shell-script and pipeline.yml header comments carried "Faster Releases"
rollout context that's only relevant while the project is in flight. Drop it,
keep the factual one-liners, and switch the build step to the :testflight:
emoji to match the matrix group.

Addresses review feedback from #25674.

* Drop transient RFC context from the TestFlight lane doc comment

The "intentionally additive / source of truth until proven" and "wired up in
later phases of the RFC" paragraphs describe the rollout state, not the lane.
Keep the build-code explanation, which documents behavior that outlives the
project.

Addresses review feedback from #25674.

* Validate BUILDKITE_BUILD_NUMBER before building the TestFlight app

The env check sat after the case statement, so a missing build number only
surfaced after update_certs_and_profiles_* had already run. Move the check to
the top of the lane to fail before any cert/profile work.

Addresses review feedback from #25674.

* Tidy the internal TestFlight upload helper

- Tag the placeholder "what's new" comment TODO instead of TBD, which isn't a
  convention anyone greps for.
- Spell the changelog env fallbacks "unknown branch" / "unknown commit" so a
  missing value reads as such.
- Manage the temp file with Dir.mktmpdir's block form, which cleans up the
  directory itself instead of a manual begin/ensure rm.

Addresses review feedback from #25674.

* Announce Reader's omission when building all apps locally

The lane is named build_all_apps_for_testflight but only builds WordPress and
Jetpack. Print a visible heads-up at runtime so someone running it locally
isn't left wondering where the Reader build went.

Addresses review feedback from #25674.

Author: jkmassel

.buildkite/commands/build-and-upload-testflight.sh

@@ -1,9 +1,6 @@
 #!/bin/bash -eu
 
 # Builds one app and uploads it to TestFlight for internal testers.
-#
-# Part of the "Faster Releases for WordPress and Jetpack" RFC. CI runs one
-# invocation per app (in parallel) via the matrix step in pipeline.yml.
 
 APP="${1:?Usage: build-and-upload-testflight.sh <wordpress|jetpack|reader>}"
 
@@ -13,5 +10,5 @@ APP="${1:?Usage: build-and-upload-testflight.sh <wordpress|jetpack|reader>}"
 echo "--- :closed_lock_with_key: Installing Secrets"
 bundle exec fastlane run configure_apply
 
-echo "--- :hammer_and_wrench: Building and uploading ${APP} to TestFlight"
+echo "--- :testflight: Building and uploading ${APP} to TestFlight"
 bundle exec fastlane build_and_upload_app_for_testflight app:"${APP}"

.buildkite/pipeline.yml

@@ -32,11 +32,7 @@ steps:
               context: "Jetpack Prototype Build"
 
   #################
-  # Continuous Delivery: build & upload each app to TestFlight (internal testers)
-  #
-  # Part of the "Faster Releases for WordPress and Jetpack" RFC. On every trunk
-  # commit, each app builds and uploads to TestFlight in parallel via the matrix
-  # below. Gated to trunk so it doesn't run on PR branches.
+  # Continuous Delivery: build & upload each app to TestFlight (internal testers) from trunk
   #################
   - group: ":testflight: TestFlight Builds"
     key: testflight_builds_group

fastlane/lanes/build.rb

@@ -317,28 +317,22 @@
   # Builds a single app and uploads it to TestFlight for *internal* testers,
   # stamping the build code with the Buildkite build number.
   #
-  # This is the per-commit "continuous delivery" build from the "Faster Releases
-  # for WordPress and Jetpack" RFC. It is intentionally additive: the existing
-  # release lanes are untouched and remain the source of truth until this flow
-  # is proven.
-  #
   # The marketing version (`VERSION_SHORT`) is read from `Version.public.xcconfig`
   # as-is. The build code is `<marketing version>.0.<buildkite build number>`
   # (e.g. `27.0.0.4567`). Buildkite build numbers increase monotonically, so each
   # build for a given marketing version gets a unique, higher build code — which
   # is all App Store Connect requires.
   #
-  # "Internal-only" means no external groups and no external-tester notifications
-  # (matching the existing Reader upload). Distributing to the a8c staff beta
-  # group, and separately to the public beta group, is wired up in later phases
-  # of the RFC.
-  #
   # @param [String] app One of `wordpress`, `jetpack`, or `reader`.
   #
   # @called_by CI
   #
   desc 'Builds one app and uploads it to TestFlight for internal testers'
   lane :build_and_upload_app_for_testflight do |app:|
+    # Fail before any cert/profile work if the build code can't be computed.
+    build_number = ENV.fetch('BUILDKITE_BUILD_NUMBER', nil)
+    UI.user_error!('BUILDKITE_BUILD_NUMBER is not set — this lane is meant to run on CI') if build_number.nil?
+
     app = app.to_s.downcase
 
     case app
@@ -367,9 +361,6 @@
       UI.user_error!("Unknown app '#{app}'. Expected one of: wordpress, jetpack, reader")
     end
 
-    build_number = ENV.fetch('BUILDKITE_BUILD_NUMBER', nil)
-    UI.user_error!('BUILDKITE_BUILD_NUMBER is not set — this lane is meant to run on CI') if build_number.nil?
-
     build_code = "#{release_version_current}.0.#{build_number}"
     UI.important("Building #{scheme} #{release_version_current} (#{build_code}) for internal TestFlight distribution")
 
@@ -419,7 +410,9 @@
   #
   desc 'Builds and uploads WordPress and Jetpack to TestFlight (internal)'
   lane :build_all_apps_for_testflight do
-    %w[wordpress jetpack].each do |app|
+    apps = %w[wordpress jetpack]
+    UI.important("Building #{apps.join(' and ')} for TestFlight. Reader is omitted because its App Store archive is currently broken.")
+    apps.each do |app|
       build_and_upload_app_for_testflight(app: app)
     end
   end
@@ -542,24 +535,22 @@ def upload_build_to_testflight(ipa_path:, whats_new_path:, distribution_groups:,
   # @param [String] beta_app_description_path Path to the beta app description file.
   #
   def upload_app_to_testflight_internal(ipa_path:, beta_app_description_path:)
-    # TBD (RFC D4): the "what's new" text for per-commit internal builds is not
+    # TODO: the "what's new" text for per-commit internal builds is not
     # finalized yet. For now, generate a minimal placeholder from the commit
     # metadata so the upload has something to show. Public-beta builds will get
     # richer notes generated from the PRs merged since the previous public build.
-    changelog = "Automated build from `#{ENV.fetch('BUILDKITE_BRANCH', 'unknown')}` (#{ENV.fetch('BUILDKITE_COMMIT', 'unknown')[0...7]})."
-    whats_new_path = File.join(Dir.tmpdir, 'testflight_whats_new.txt')
+    changelog = "Automated build from `#{ENV.fetch('BUILDKITE_BRANCH', 'unknown branch')}` (#{ENV.fetch('BUILDKITE_COMMIT', 'unknown commit')[0...7]})."
 
-    begin
+    Dir.mktmpdir do |dir|
+      whats_new_path = File.join(dir, 'testflight_whats_new.txt')
       File.write(whats_new_path, changelog)
 
       upload_build_to_testflight(
         ipa_path: ipa_path,
         whats_new_path: whats_new_path,
-        distribution_groups: [], # Internal-only for now (RFC D2): no external groups.
+        distribution_groups: [],
         beta_app_description_path: beta_app_description_path
       )
-    ensure
-      FileUtils.rm_rf(whats_new_path)
     end
   end