-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Add hashableModels to additional properties #9495
Add hashableModels to additional properties #9495
Conversation
This reverts commit 18053af.
977243a
to
a0bce08
Compare
a0bce08
to
e525215
Compare
@maoyama Thanks for opening this PR. But could you please rename the option |
- Replace hashableStruct => hashableModels - Replace HashableStruct => HashableModels - Replace HASHABLE_STRUCT => HASHABLE_MODELS - Update docs
@4brunu You’re right. |
@@ -1,4 +1,4 @@ | |||
{{^objcCompatible}}{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} {{#useClasses}}final class{{/useClasses}}{{^useClasses}}struct{{/useClasses}} {{classname}}: Codable, Hashable { | |||
{{^objcCompatible}}{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} {{#useClasses}}final class{{/useClasses}}{{^useClasses}}struct{{/useClasses}} {{classname}}: Codable{{#vendorExtensions.x-swift-hashable}}, Hashable{{/vendorExtensions.x-swift-hashable}} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maoyama shouldn't we also check here for {{#hashableModels}}...{{/hashableModels}}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4brunu We don't have to check here.
I set x-swift-hashable to true when hashableModels is true to reduce the logic in modelObject.mustache. fb2c45f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, good idea! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Looks good to me, let's wait for the CI to finish |
CI is failing, but it's not related to this PR. |
* Add hashableStruct * Revert "Remove x-swift-hashable" This reverts commit 18053af. * Add Hashable for x-swift-hashable * Add config yaml to test x-swift-hashable * Run ./bin/generate-samples.sh ./bin/configs/swift5* * Run ./bin/utils/export_docs_generators.sh * Run ./bin/generate-samples.sh ./bin/configs/swift5* * Verify setHashableStruct * Rename hashableStruct => hashableModels - Replace hashableStruct => hashableModels - Replace HashableStruct => HashableModels - Replace HASHABLE_STRUCT => HASHABLE_MODELS - Update docs * Refactor modelObject.mustache * Control equals and hash functions
Resolves #9473 , Related #9166
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
,5.1.x
,6.0.x
@jgavris (2017/07) @ehyche (2017/08) @Edubits (2017/09) @jaz-ah (2017/09) @4brunu (2019/11)