-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ref(appstore-connect): Stop asking users to refresh their iTunes credentials [NATIVE-294] #29654
Conversation
size-limit report
|
iTunesSessionInvalid: t( | ||
'The iTunes session of your configured App Store Connect needs to be refreshed.' | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to keep the PR focused on just removing code that's no longer being used, the only change here is to delete this scenario and where it's being used.
however, this change actually leaves this map with only one entry. it may make sense in a different PR to revisit the code in this file and to re-evaluate whether it still makes sense the way it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure I can revisit this code as soon we merge this PR
bdf59bd
to
6eb20e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good to me
iTunesSessionInvalid: t( | ||
'The iTunes session of your configured App Store Connect needs to be refreshed.' | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure I can revisit this code as soon we merge this PR
@@ -493,7 +481,6 @@ def get(self, request: Request, project: Project, credentials_id: str) -> Respon | |||
"latestBuildVersion": latestBuildVersion, | |||
"latestBuildNumber": latestBuildNumber, | |||
"lastCheckedBuilds": last_checked_builds, | |||
"promptItunesSession": bool(pending_downloads and itunes_session_info is None), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
heh, i would have gone for hardcoding this to False as that didn't need the client code to be updated at all. but i guess fully removing it is cleaner :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fewer lines of code! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for deleting client code <3
6eb20e0
to
7494490
Compare
7494490
to
fd83432
Compare
fd83432
to
23ad1b8
Compare
23ad1b8
to
9766676
Compare
#29513 begins work that removes the last dependency that the App Store Connect integration has on iTunes. This PR removes all of the code related to alerting the user about outdated iTunes credentials as they should no longer be used for anything.
Warnings related to incorrect, invalid, or outdated App Store Connect credentials are preserved.
related: #29531