Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Custom mappings between schema formats and Swift Types#142

Merged
liamnichols merged 7 commits intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom
LePips:datatype-swift-type-overrideLePips/CreateAPI:datatype-swift-type-overrideCopy head branch name to clipboard
Aug 15, 2022
Merged

Custom mappings between schema formats and Swift Types#142
liamnichols merged 7 commits intoCreateAPI:mainCreateAPI/CreateAPI:mainfrom
LePips:datatype-swift-type-overrideLePips/CreateAPI:datatype-swift-type-overrideCopy head branch name to clipboard

Conversation

@LePips
Copy link
Contributor

@LePips LePips commented Aug 15, 2022

This PR allows custom Swift type overrides for schema datatype formats (not global for this):

datatypes:
  string:
    uuid: String
    password: MyPasswordType
  integer:
    int32: Int
    int64: Int

Also removes useFixWidthIntegers in favor of the above example datatype override.

@LePips
Copy link
Contributor Author

LePips commented Aug 15, 2022

@liamnichols

Because useFixWidthIntegers was removed all of the tests need to be updated because its default was true. So, would you like me to either:

1 - update the tests as newly generated with Int32/Int64
Very large diff, but upholds default behavior. Test to be added would be overriding datatypes

2 - update the test definitions with int32/int64 overrides
Smaller diff, but all test datatype overrides. Test to be added would be a plain test for default behavior

@liamnichols
Copy link
Member

Good point! Let's preserve the original behaviour first (use a default of Int for both of the formats) and then we can sort the default later 👍🏼

Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Looking good so far! Yeah let's preserve the original default behaviour for now, then we can change it in a follow up PR 👍🏼

Sources/CreateAPI/Generator/Generator+DataTypes.swift Outdated Show resolved Hide resolved
Sources/CreateOptions/ConfigOptions.swift Show resolved Hide resolved
@LePips
Copy link
Contributor Author

LePips commented Aug 15, 2022

To accommodate the smallest diff here for "preserving the default behavior" I force int32/int64 to be Int which only requires one test case to be updated but will be removed.

Commented TODO at the top of Generator+DataTypes for next step. Updating the tests will be a big diff also due to us not previously respecting float.

@LePips LePips requested a review from liamnichols August 15, 2022 15:11
@LePips
Copy link
Contributor Author

LePips commented Aug 15, 2022

In my documentation I state that types must conform to Codable. I assume this because I think we can make the decision to remove the options skipRedundantProtocols, alwaysIncludeDecodableImplementation, and alwaysIncludeEncodableImplementation unless there is something I am missing that require this behavior.

If there is no other motive for these options, I just think that these are kind of redundant as it's implicitly expected that objects are Decodable+Encodable from/to network calls. Codable conformance is default and can be removed manually if desired as always.

Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Briliant work! 🚀

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.

Support custom mappings between schema formats and Swift types

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.