-
Notifications
You must be signed in to change notification settings - Fork 110
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
activesupport: update to_xml
signature to use an optional block
#741
activesupport: update to_xml
signature to use an optional block
#741
Conversation
@hjwylde Thanks for your contribution! Please follow the instructions below for each change. Available commandsYou can use the following commands by commenting on this PR.
|
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.
APPROVE
# </message> | ||
# </messages> | ||
# | ||
def to_xml: (?::Hash[untyped, untyped] options) ?{ (untyped) -> untyped } -> untyped |
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.
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! Just to clarify, are these notes just for references to the relevant code, or review points?
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.
After reading the other reviews I believe they are just references, so I'll merge this one too, thank you.
# The default XML builder is a fresh instance of <tt>Builder::XmlMarkup</tt>. You can | ||
# configure your own builder with the <tt>:builder</tt> option. The method also accepts | ||
# options like <tt>:dasherize</tt> and friends, they are forwarded to the builder. | ||
def to_xml: (?::Hash[untyped, untyped] options) ?{ (untyped) -> untyped } -> untyped |
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.
/merge |
Split from #735 to be smaller. Type signature updates for invalid signatures in activesupport, due to blocks being marked as required, but they should be optional.