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

[cli] refactor IQuit, contextual errors #1891

Merged
merged 3 commits into from
Apr 16, 2020
Merged

Conversation

quinlanj
Copy link
Member

why

  • refactor IQuits to classes
  • if view errors and our Quit interface is AskQuit - show that view
  • if view errors and our Quit interface is DoQuit - propagate error up to caller
  • add contextual build errors to IOSBuilder

related

#1881

TODO

land expo/fyi#4

@@ -123,6 +124,15 @@ See https://docs.expo.io/versions/latest/distribution/building-standalone-apps/#
}
await this.produceCredentials(context, experienceName, bundleIdentifier);
} catch (e) {
if (e.code === ErrorCodes.NON_INTERACTIVE) {
const here = terminalLink('here', 'https://expo.fyi/credentials-non-interactive');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const here = terminalLink('here', 'https://expo.fyi/credentials-non-interactive');
const here = terminalLink('expo.fyi/credentials-non-interactive', 'https://expo.fyi/credentials-non-interactive');

Copy link
Member

Choose a reason for hiding this comment

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

i know i suggested something different but i think this maybe makes it stand out a bit more!

@brentvatne
Copy link
Member

brentvatne commented Apr 16, 2020

thanks! this is an improvement

if it's not too much trouble it would be nice in a followup pr to not show any text after the error msg, so:

╭─~/code/loadcsv ‹master›
╰─$ local-expo build:ios --non-interactive
[20:33:01] Checking if there is a build in progress...

[20:33:02] Fetching available credentials
[20:33:03] Unable to validate distribution certificate due to insufficient Apple Credentials
[20:33:03] Unable to validate Push Keys due to insufficient Apple Credentials
[20:33:03] Additional information needed to setup credentials in non-interactive mode.
[20:33:03] Learn more about how to resolve this: expo.fyi/credentials-non-interactive
[20:33:03] Failed to prepare all credentials.
The next time you build, we will automatically use the following configuration:
[20:33:03]
[20:33:03] Project Credential Configuration:
[20:33:03]   Experience: @notbrent/loadcsv-unused-slug, bundle identifier: xyz.bront.app
[20:33:03]     Provisioning profile is missing. It will be generated during the next build
[20:33:03]
[20:33:03]   Distribution Certificate - Certificate ID: -----
[20:33:03]     Apple Team ID: --,  Apple Team Name: ---------
[20:33:03]     used by
      @notbrent/loadcsv-unused-slug (xyz.bront.app)
[20:33:03]   Push Notifications Key - Key ID: ---
[20:33:03]     Apple Team ID: --,  Apple Team Name: ---------
[20:33:03]     used by
      @notbrent/loadcsv-unused-slug (xyz.bront.app)
[20:33:03] Input is required, but Expo CLI is in non-interactive mode.
Required input:
> Will you provide your own Apple Provisioning Profile?

this should become

╭─~/code/loadcsv ‹master›
╰─$ local-expo build:ios --non-interactive
[20:33:01] Checking if there is a build in progress...

[20:33:02] Fetching available credentials
[20:33:03] Unable to validate distribution certificate due to insufficient Apple Credentials
[20:33:03] Unable to validate Push Keys due to insufficient Apple Credentials
[20:33:03] Additional information needed to setup credentials in non-interactive mode.
[20:33:03] Learn more about how to resolve this: expo.fyi/credentials-non-interactive

the issue with the first version is that it can be hard for people to pick apart which error actually matters to them. if we know for certain what the error is that is preventing them from continuing and that is all that we know, then we should show that only.

@brentvatne brentvatne merged commit 57abd9f into master Apr 16, 2020
@brentvatne brentvatne deleted the @quin/refactorQuit branch April 16, 2020 04:07
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.

2 participants