Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

[expo-cli] use Submission Service for Android uploads #2069

Merged
merged 40 commits into from
May 26, 2020

Conversation

dsokal
Copy link
Contributor

@dsokal dsokal commented May 5, 2020

This PR integrates expo-cli with Submission Service. Submission Service enables users to perform app submissions from Expo Servers. However, only Android submissions are available at the moment. The iOS part will be added soon.

The command for submitting apps stays the same - expo upload:android. By default, this command will try to run fastlane on the user's machine - this is expected behavior. Some users might want to submit their apps the old way - without using Submission Service.
In order to use Submission Service, one needs to pass the --use-submission-service flag.

This PR also adds more features to expo upload:android:

  • works on all operating systems! (if the user passes --use-submission-service)
    • offline uploads are still limited to macOS
  • solutions to common errors - if the user passes --use-submission-service, the submission fails and we've managed to detect what went wrong - we're printing a link to an expo.fyi page with the solution to that problem
  • --url parameter - for specifying the archive's url
    • it makes it possible to use this command outside of the project dir - we'll add this feature in the future
    • other ways of choosing the archive stayed the same: --latest (the last build from Expo servers), --id (a build identified by a build id)
  • --type [apk|aab] parameter - if the archive url/path doesn't contain the filename extension then it's possible to specify the archive type
  • --release-status [completed|draft|halted|inProgress] parameter - to specify the release status of the app in Google Play Store
  • --verbose flag (only for Submission Service) - by default, we don't print logs from Submission Service; this flag enables the user to show them anyway

Next steps:

  • Start using Submission Service by default. We need to come up with a way of rolling out this feature to users. This requires to implement some logic in www. We don't want to turn this on for all users at once. This will enable us to ultimately get rid of the --use-submission-service flag - we'll have an opposite one - --offline (or similar) for submitting apps from the user's machine.
  • Implement iOS part of the Submission Service and integrate it with expo-cli.

To do before merging:

To do after merging:

@codecov-io
Copy link

codecov-io commented May 5, 2020

Codecov Report

Merging #2069 into master will not change coverage.
The diff coverage is n/a.

Flag Coverage Δ
#config 61.33% <0.00%> (ø)
#devServer 60.53% <0.00%> (ø)
#expoCli 35.78% <0.00%> (ø)
#expoCodemod 83.81% <0.00%> (ø)
#jsonFile 65.49% <0.00%> (ø)
#packageManager 16.67% <0.00%> (ø)
#plist 70.64% <0.00%> (ø)
#pwa 34.32% <0.00%> (ø)
#schemer 69.88% <0.00%> (ø)
#uriScheme 34.70% <0.00%> (ø)
#webpackConfig 46.68% <0.00%> (ø)
#xdl 5.94% <0.00%> (ø)
@@           Coverage Diff           @@
##           master    #2069   +/-   ##
=======================================
  Coverage   36.01%   36.01%           
=======================================
  Files         148      148           
  Lines        6937     6937           
  Branches     1604     1604           
=======================================
  Hits         2498     2498           
  Misses       3249     3249           
  Partials     1190     1190           

@dsokal dsokal force-pushed the @dsokal/integrate-ss branch 5 times, most recently from 546c1eb to b445e95 Compare May 11, 2020 14:48
@dsokal dsokal requested a review from brentvatne May 11, 2020 15:06
@dsokal dsokal changed the title integrate with Submission Service [expo-cli] use Submission Service for Android uploads May 11, 2020
Comment on lines 127 to 140
message: 'What would you like to submit?',
choices: [
{ name: 'I have a url to the app archive', value: ArchiveSourceType.url },
{
name: "I'd like to upload the app archive from my computer",
value: ArchiveSourceType.path,
},
{
name: 'The lastest build from Expo Servers',
value: ArchiveSourceType.latest,
},
{
name: 'A build identified by a build id',
value: ArchiveSourceType.buildId,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestions for better/more meaningful messages are welcome :)

Comment on lines 185 to 220
async function askForArchiveUrlAsync(): Promise<string> {
const { url } = await prompt({
name: 'url',
message: 'URL:',
type: 'input',
validate: (url: string): string | boolean =>
validateUrl(url) || `${url} does not conform to HTTP format`,
});
return url;
}

async function askForArchivePathAsync(): Promise<string> {
const { path } = await prompt({
name: 'path',
message: 'Path to the app archive file:',
type: 'input',
validate: async (path: string): Promise<boolean | string> => {
if (!(await existingFile(path, false))) {
return `File ${path} doesn't exist.`;
} else {
return true;
}
},
});
return path;
}

async function askForBuildIdAsync(): Promise<string> {
const { id } = await prompt({
name: 'id',
message: 'Build ID:',
type: 'input',
validate: (val: string): boolean => val !== '',
});
return id;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@dsokal dsokal force-pushed the @dsokal/integrate-ss branch 3 times, most recently from 67f4759 to 23da17e Compare May 21, 2020 08:05
@dsokal dsokal requested a review from wkozyra95 May 21, 2020 15:00
1
);
if (builds.length === 0) {
log.error("Couldn't find any builds for this project.");
Copy link
Member

@brentvatne brentvatne May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be more clear about what this means and follow guidelines as mentioned in #2165 (probably use bold text)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this command won't exit if this log is printed. See the line below - a prompt will appear.
However, I'll try to make this message a bit more clear.

name: 'url',
message: 'URL:',
type: 'input',
validate: (url: string): string | boolean =>
Copy link
Member

@brentvatne brentvatne May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can make the default value here something like https://path.to/your/archive.aab and when we validate we just ensure that they didn't give us that exact value.

like: "That was just an example URL, meant to show you the format that we expect for the response."

Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few feedback points that i hope will be helpful. i also ran the command manually a few more times and everything worked very well!

if (process.platform !== 'darwin') {
log.error('Unsupported platform! This feature works on macOS only.');
if (targetPlatform === 'android') {
Copy link
Member

@brentvatne brentvatne May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 this is great that we're letting users know about the flag contextually when attempting to run an upload from an unsupported platform

} = this.submissionConfig;

// TODO: check if `fastlane supply` works on linux
const travelingFastlane = require(`@expo/traveling-fastlane-${os.platform()}`)();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For windows we should use @expo/traveling-fastlane-linux

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also some better error message could be useful here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed that. Regarding the error message - there is already a meaningful error message - https://github.com/expo/expo-cli/pull/2069/files#diff-911f5cceaa622faf6d5d53c7b83e5d3dR128 Generally, this code will never be executed on a different platform than macOS (yet).

@@ -0,0 +1,41 @@
import { AndroidSubmissionConfig } from './android/AndroidSubmissionConfig';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we use a naming convention like that anywhere else?
SubmissionService.types.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do:
Screenshot 2020-05-26 at 11 20 09

@dsokal dsokal merged commit dd058da into master May 26, 2020
@dsokal dsokal deleted the @dsokal/integrate-ss branch May 26, 2020 09:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants