Skip to content

Conversation

@grabbou
Copy link
Member

@grabbou grabbou commented Mar 18, 2020

Summary:

Fixes #1060

Test Plan:

Run pod install on outdated repo. Also includes a mechanism to prevent loops in case pod repo update didn't fix the error (e.g. invalid version specified).

@grabbou grabbou changed the title feat: run pod repo when pod install failed feat: run pod repo update when pod install failed Mar 18, 2020
@grabbou
Copy link
Member Author

grabbou commented Mar 18, 2020

With this PR applied, I was able to successfully continue manual tests of the upcoming release.

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

LGTM

await runPodInstall(loader, projectName, false);
} else {
loader.fail();
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

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

Let's use CLIError and pass the original error so that issues like this one are easier to deal with. Would be cool to do the same for runPodUpdate function (line 68)

Copy link
Member Author

Choose a reason for hiding this comment

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

Was just copying and pasting the existing code and actually started thinking why we no longer use CLIError.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out this will take more than should be in scope of this PR and I guess we should track it separately.

CLIError will inline the entire message into a single line + we already logger.error here (in other CLI commands, we don't do that as we have global try-catch).

I would be a bit nervous about changing it here due to potential side effects. What you think of setting up a follow up issue?

Copy link
Member

Choose a reason for hiding this comment

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

I'm good with a followup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically handle CocoaPods being out of date while creating a new project

3 participants