[app store connect] cleanup token creation code across entire project #18186
Conversation
joshdholtz
changed the base branch from
master
to
joshdholtz-new-config-option-for-multiple-env-names
joshdholtz
changed the base branch from
joshdholtz-new-config-option-for-multiple-env-names
to
joshdholtz-unified-app-store-connect-api-key-env
| @@ -2,10 +2,6 @@ | |||
|
|
|||
| module Fastlane | |||
| module Actions | |||
| module SharedValues | |||
| APP_STORE_CONNECT_API_KEY = :APP_STORE_CONNECT_API_KEY | |||
There was a problem hiding this comment.
do we really want to break API for users who might depend on this?
There was a problem hiding this comment.
I was unsure about this. I didn’t super want to keep it but I also didn’t want to break 😬 I can put it back in!
match/lib/match/importer.rb
Outdated
| @api_token ||= Spaceship::ConnectAPI::Token.create(params[:api_key]) if params[:api_key] | ||
| @api_token ||= Spaceship::ConnectAPI::Token.from_json_file(params[:api_key_path]) if params[:api_key_path] | ||
| return @api_token | ||
| api_token = Spaceship::ConnectAPI::Token.from(hash: params[:api_key], filepath: params[:api_key_path]) |
There was a problem hiding this comment.
in this class (and a few below) we are not consistent with the way we log about wether we reuse or create a token.
For example the implementation in fastlane/lib/fastlane/actions/set_changelog.rb treats them differently
There was a problem hiding this comment.
match has a bunch of tools that are only called from the CLI as sub commands. So this one should never really have token to reuse already 🤷♂️ But I can make these look and reuse a token if there is an existing one!
There was a problem hiding this comment.
lol, I read your original comment wrong 😛 I will make this change!
joshdholtz
force-pushed
the
joshdholtz-unified-app-store-connect-api-key-env
branch
from
28cf78b
to
5b32701
Compare
joshdholtz
force-pushed
the
joshdholtz-cleanup-asc-token-creation-in-each-action
branch
from
a1a9881
to
1cec4b5
Compare
| @@ -13,7 +13,6 @@ def self.run(params) | |||
|
|
|||
| begin | |||
| Cert.config = params # we alread have the finished config | |||
| Cert.config[:api_key] ||= Actions.lane_context[SharedValues::APP_STORE_CONNECT_API_KEY] | |||
There was a problem hiding this comment.
Not needed anymore because Cert will look for existing Spaceship::ConnectAPI.client now and use that
| @@ -10,7 +10,6 @@ def self.run(params) | |||
| require 'match' | |||
|
|
|||
| params.load_configuration_file("Matchfile") | |||
| params[:api_key] ||= Actions.lane_context[SharedValues::APP_STORE_CONNECT_API_KEY] | |||
There was a problem hiding this comment.
Not needed anymore because Match will look for existing Spaceship::ConnectAPI.client now and use that
| if values[:api_key_path].nil? | ||
| values[:api_key] ||= Actions.lane_context[SharedValues::APP_STORE_CONNECT_API_KEY] | ||
| end |
There was a problem hiding this comment.
Not needed anymore because Pilot will look for existing Spaceship::ConnectAPI.client now and use that
|
@lacostej I think this is ready for a re-review (finally 😅) |
…asc-token-creation-in-each-action
joshdholtz
deleted the
joshdholtz-cleanup-asc-token-creation-in-each-action
branch
There was a problem hiding this comment.
Congratulations! 🎉 This was released as part of fastlane 2.184.0 🚀
Motivation and Context
Adds on to #18173
Partly resolves #18145
Description
app_store_connect_api_keyactionSharedValues::APP_STORE_CONNECT_API_KEYSpaceship::ConnectAPI.token