-
Notifications
You must be signed in to change notification settings - Fork 79
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] Add AppStore app creation to submission command #65
Conversation
2a8c2ed
to
4884010
Compare
we should use the term app config instead:
we can use terminal-link to link out to the docs. |
will follow up with a more thorough review next week :) one other thing we may want to do here is move this command out of the |
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.
Great job, take a look at my inline comments!
'bundle-identifier': flags.string({ | ||
description: 'Your iOS Bundle Identifier (default: expo.ios.bundleIdentifier from app.json)', | ||
}), |
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.
Hmm, I'm not sure if this should be a command param. I'm not saying it's undesirable but rather we don't have a similar param for build commands.
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.
Hmm, my current flow assumed:
- Get bundle ID from this param
- Then try to gather it from exp config
- Finally, prompt user
But according to the comment below (about using getBundleIdentifier()
), the flow can be changed to just gather it from param or config (remove prompt).
Providing bundle ID as param / asking for it is useful only in cases when user wants to submit an external binary and/or, if ever possible, to run eas submit
from outside project directory (manifest not available).
I'm in favor of |
Proper behavior of terminal link should be that if it's supported it should display text and if it's not display URL, but there are terminals that display text and it's not clickable. termianl-link has |
That's the exact reason why I didn't use |
c6ea47d
to
2aca849
Compare
which terminals are you concerned about? we use terminal-link extensively in expo-cli and intend to continue to use it in our cli tools so we should probably make this a broader discussion if there is some concern, rather than diverging on a per-command basis :) (this is not a blocker for this PR, just something we should discuss at some point to agree on a common strategy for these types of links) |
@brentvatne |
Added App Store Connect app creation, currently still depending on
traveling-fastlane
Command flow
This is how the command flow currently works:
appName
andbundleIdentifier
from flags first, then fromapp.json
, eventually promptExample output
Click to expand
Also done
eas build:submit
toeas submit
, leaving alias for the first one.TODO (future PR)
travelingFastlane
with JS API after it supports iTunes teams/providers.Find a name forRenamed to--app-apple-id
--asc-app-id
Note: I'm repeating myself but this name is really confusing. The
![Screenshot 2020-11-13 at 07 37 56](https://user-images.githubusercontent.com/278340/99038534-ee128000-2585-11eb-87ad-e62d01340fb0.png)
--asc-app-id
param is corresponding toappAppleId
variable, on ASC it's this one:It's in App Store Connect under General ->App Information