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: Adding back customType with deprecated message #562

Merged
merged 4 commits into from
Jun 18, 2020

Conversation

lawmicha
Copy link
Contributor

Issue #, if available:

Description of changes:

  • Add a customType to allow compiling previous code generated code by Amplify CLI versions < 4.21.4

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

@lawmicha lawmicha requested review from palpatim and drochetti June 18, 2020 17:42
@lawmicha lawmicha changed the title bug: Adding back customType with deprecated message fix: Adding back customType with deprecated message Jun 18, 2020
@lawmicha lawmicha self-assigned this Jun 18, 2020
Podfile.lock Outdated
https://cdn.cocoapods.org/:
trunk:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the CDN is the right one. Something tells me you have an outdated setting in your Cocoapod installation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's odd, tried with pod repo remove trunk and pod install on fresh project. trunk gets added to the repo list every time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will remove this change for now to get this change in first, will take a look at this later when doing the release commit

Copy link
Member

Choose a reason for hiding this comment

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

I believe this changes back & forth based on the local workstation's cocoapods setup, but it should be innocuous as long as we're all on sufficiently recent versions of CocoaPods (> 1.8.4 IIRC).

@@ -33,6 +33,14 @@ public enum ModelFieldType {
}
}

@available(*, deprecated, message: """
This has been replaced with `.embedded(type)` and `.embeddedCollection(of)`
Copy link
Member

Choose a reason for hiding this comment

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

nit: add a trailing backslash to avoid line breaks:

...and `.embeddedCollection(of)` \
Please use...

@@ -33,7 +33,7 @@ extension Todo {
.field(todo.name, is: .required, ofType: .string),
.field(todo.description, is: .optional, ofType: .string),
.field(todo.categories, is: .optional, ofType: .embeddedCollection(of: Category.self)),
.field(todo.section, is: .optional, ofType: .embedded(type: Section.self)),
.field(todo.section, is: .optional, ofType: .customType(type: Section.self)),
Copy link
Member

Choose a reason for hiding this comment

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

Is this right? Shouldn't it be embedded?

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 was using this to test that using .customType compiles. actually let me add another class entirely and put a comment on Amplify CLI version, and deprecated note

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@lawmicha lawmicha force-pushed the lawmicha/customtype branch from 470dd5b to 7b4ae25 Compare June 18, 2020 20:21
@lawmicha lawmicha requested review from palpatim and drochetti June 18, 2020 20:35
@lawmicha lawmicha merged commit d3ba16a into master Jun 18, 2020
@lawmicha lawmicha deleted the lawmicha/customtype branch June 18, 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