Skip to content

Conversation

ejithon
Copy link
Contributor

@ejithon ejithon commented Feb 13, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

#6941

Add explicit public initializer again for modelObject.mustache.

Initializers are no longer see from other Xcode targets after merged the above issue. Swift struct can omit explicit initializer, but is necessary if reference it from outer packages.

In my case, I usually generate a CocoaTouch Framework from generated codes and reuse it via Carthage.

@wing328
Copy link
Contributor

wing328 commented Feb 18, 2018

@ejithon thanks for the PR. cc technical committee for review:

@jgavris (2017/07) @ehyche (2017/08) @Edubits (2017/09) @jaz-ah (2017/09)

(the CircleCI failure can be ignored)

@fl034
Copy link

fl034 commented Feb 23, 2018

I found the same issue while testing 2.4.0 with the new struct models.

From the Apple Docs:

Default Memberwise Initializers for Structure Types
The default memberwise initializer for a structure type is considered private if any of the structure’s stored properties are private. Likewise, if any of the structure’s stored properties are file private, the initializer is file private. Otherwise, the initializer has an access level of internal.
As with the default initializer above, if you want a public structure type to be initializable with a memberwise initializer when used in another module, you must provide a public memberwise initializer yourself as part of the type’s definition.

Thus we have to provide an initializer. Memberwise synthesized initializers are set to internal which leads to the fact that they're invisible if the generated client is not in the same module.

@fl034
Copy link

fl034 commented Feb 26, 2018

I tested your changes and they're pretty well working 🎉 . Thanks!

@d-date
Copy link
Contributor

d-date commented Mar 7, 2018

I think this is make sense. when we converted from class to struct, maybe we forget that memberwise initializer in public struct disappear. unless we merged, they cannot initialize struct from outside.

@wing328 This PR should be merged ASAP. Could you merge this PR?

@d-date
Copy link
Contributor

d-date commented Mar 12, 2018

cc @jgavris @ehyche @Edubits @jaz-ah

{{/isEnum}}
{{/allVars}}

{{#hasVars}}
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: should be as below for indent

{{/allVars}}

{{#hasVars}}
    public init({{#allVars}}{{name}}: {{{datatypeWithEnum}}}{{^required}}?{{/required}}{{#hasMore}}, {{/hasMore}}{{/allVars}}) {
        {{#allVars}}
        self.{{name}} = {{name}}
        {{/allVars}}
    }
{{/hasVars}}
{{#additionalPropertiesType}}

@wing328
Copy link
Contributor

wing328 commented Mar 15, 2018

The change looks reasonable to me. Merging it into master.

@wing328 wing328 merged commit e22faf4 into swagger-api:master Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants