-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Swift: add more doc strings to generated things #14715
Conversation
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.
LGTM (I checked the generation code and a sample of the generated comments). These are a bit repetitive, but I think will really help a reader who hasn't figured out what each type of file is for already.
@@ -1,4 +1,10 @@ | |||
// generated by {{generator}}, remove this comment if you wish to edit this file | |||
/** | |||
* This module provides a hand-modifiable wrapper around the generated class `{{name}}`. | |||
{{#ql_internal}} |
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.
I'm not sure ql_internal
is part of the Stub
?
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.
good catch! And it made me think, that all the generated classes should actually be marked INTERNAL
, as they should never be used directly, so I changed that as well.
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.
But methods from generated classes are used all the time and thus should not be marked as internal, right? For example Callable.getName()
is from Generated::Callable
.
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.
exactly. Unless ql.internal
is used on a specific property (for example kind: int | ql.internal
), getters are not marked as internal.
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.
Looks all good now. 👍
I was expecting a minor merge conflict with #14261 , but I can't see any evidence this happened.
The conflict was there on |
Also, all files under
generated
and their classes are now markedINTERNAL
regardless of theql.internal
pragma, as they should never be used directly (a user should not writeGenerated::Bla
).