-
Notifications
You must be signed in to change notification settings - Fork 93
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
[eas-cli] use new android build credentials provider #427
Conversation
Size Change: +473 B (0%) Total Size: 37.5 MB
|
projectName: ctx.commandCtx.projectName, | ||
accountName: ctx.commandCtx.accountName, | ||
androidApplicationIdentifier: nullthrows( |
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.
This value from app.json is not correct for generic projects. Actually, we even print a warning if someone is building a generic project and they have android.package
defined in their app.json (see https://github.com/expo/eas-cli/blob/main/packages/eas-cli/src/project/android/applicationId.ts#L35). For generic, we read the application id from native code.
Take a look at line 70 in this file and use the value returned by the getOrConfigureApplicationIdAsync
function.
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.
good catch, thanks 🙏
@@ -113,14 +115,22 @@ async function ensureAndroidCredentialsAsync( | |||
if (!shouldProvideCredentials(ctx)) { | |||
return; | |||
} | |||
const androidApplicationIdentifier = await getOrConfigureApplicationIdAsync( |
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.
const androidApplicationIdentifier = await getOrConfigureApplicationIdAsync( | |
const androidApplicationIdentifier = getApplicationId( |
This change is related to #431 (comment)
The application id must be already configured in this context.
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.
Sorry for making you change it again but in https://github.com/expo/eas-cli/pull/427/files/887877aaf9874f6b77a09f3c8632456d54f98355#r640438388 I meant to use the result of the call on line 70.
However, when I was reviewing #431 I realized that we should change the name (and behavior) of getOrConfigureApplicationIdAsync
😅
23c391b
to
9ac5688
Compare
Checklist
[EAS BUILD API]
if it's a part of a breaking change to EAS Build API (only possible when updating@expo/eas-build-job
package).Why
Replace the old Android SetupBuildCredentials with the new one
Test Plan