[spaceship] TestFlight::BuildTrains is a collection modelled as a Spaceship::Base
Spaceship::TestFlight::BuildTrains inherits Spaceship::Base but holds a Hash of version string => builds, not a parsed Apple payload. It is the only subclass of Base in spaceship that overrides initialize, and it does not call super, so nothing Base#initialize sets ever happens.
Every method it inherits is dead or wrong on a live instance:
| method | value |
|---|---|
raw_data |
nil |
client |
nil |
attributes |
[] |
setup |
never runs |
inspect / to_s |
<Spaceship::TestFlight::BuildTrains \n> |
to_json |
"null" |
to_json is the only one actively wrong rather than empty: TestFlight::Base#to_json is raw_data.to_json, and nil.to_json is "null", so a collection of build trains serialises as null.
The class side is unused too — no attr_mapping is declared, and BuildTrains.all goes to Spaceship::ConnectAPI::Build.all rather than through the inherited client.
What the class adds over a Hash is get/[], values and versions — Hash#[], Hash#values and Hash#keys. Both callers in the repo use only the last two:
trains.values.flatten # test_flight/build.rb:111
self.build_trains(platform: ...).versions # tunes/application.rb:312Spaceship does this differently elsewhere. Portal::Persons is a plural with class methods only — Persons.all returns a plain Array and nothing instantiates Persons. Tunes::BuildTrain.all likewise returns a plain Hash. BuildTrains is the only plural in spaceship that instantiates itself.
Proposal
Make it Persons-shaped: class methods only, all returns the Hash it already builds.
This is what Application#build_trains documents itself as returning:
# TestFlight: A reference to all the build trains
# @return [Hash] a hash, the version number and platform being the key
def build_trains(platform: nil)[] and values keep working, as Hash methods. versions and get disappear; get has no caller, and the one caller of versions is Application#all_build_train_numbers, which would use .keys and keep its own signature. So nothing in fastlane's public surface changes.
The residual risk is external code calling BuildTrains.all(...).versions directly, which is calling versions on an object documented as a Hash.
That can be avoided by returning a Hash subclass that keeps versions and get as deprecated delegators to keys and [].
Two smaller alternatives, if that is judged too invasive:
- Leave the inheritance and give the class a working
to_json(@trains.to_json). Removes the only actively wrong behaviour and changes nothing else. - Drop the
Baseinheritance but keep the wrapper. Nothing in the repo depends on it — there are nois_a?(Spaceship::Base)checks anywhere — andinspectimproves.to_jsonwould raise unless added back.
Context: #30238 sharpens the Base#initialize comment this class is the counter-example to.
Source: fastlane/fastlane