Skip to content
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

fix(tools): change the tools script to comply with amplify-app changes #445

Merged
merged 2 commits into from
May 14, 2020

Conversation

drochetti
Copy link
Contributor

See aws-amplify/amplify-cli#4268 for details

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@drochetti drochetti requested review from palpatim and wooj2 May 14, 2020 19:12
trunk:
https://cdn.cocoapods.org/:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@palpatim is this expected/correct?

Copy link
Member

Choose a reason for hiding this comment

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

Yep!

AmplifyTools/amplify-tools.sh Show resolved Hide resolved
@@ -25,7 +25,10 @@ amplifyRegion=$region
amplifyEnvName=$envName

if $amplifyModelgen; then
Copy link
Contributor

Choose a reason for hiding this comment

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

not familiar with this syntax, i would have expected to check if amplifyModelgen is not set by using:

if [[ ! -z $amplifyModelgen ]]; then

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'm not familiar either tbh

@@ -24,7 +24,7 @@ DEPENDENCIES:
- SwiftLint

SPEC REPOS:
trunk:
https://cdn.cocoapods.org/:
Copy link
Contributor

Choose a reason for hiding this comment

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

per tim's comment above -- same with this.. is this expected? i would have probably reverted this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

me too, but why did my cocoapods make that change? If that's not supposed to happen then I'm very confused 😕

Copy link
Member

Choose a reason for hiding this comment

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

@drochetti drochetti merged commit 67412ca into master May 14, 2020
@drochetti drochetti deleted the fix/amplify-tools-xcode-groups branch May 14, 2020 22:14
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.

3 participants