-
Notifications
You must be signed in to change notification settings - Fork 15
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
Document SAP system aggregate #988
Conversation
18eec36
to
c522d2d
Compare
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.
Just some really tiny suggestions, awesome job man
Thanks @dottorblaster , |
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
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.
Great work! Just a nitpick and then go!
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.
Gonna read this many times 😛
Good job!
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 :D Everything is clear and understandable from the first read.
Document SAP system aggregate code. I have included some additional documentation in the SAP application instance registering command, to explain how the application and database are associated.
This part of the code is a bit hidden, but I hope the new docs help a bit on that.
PD: We cannot put docs in the protocol code as it is a
defimpl
, that's why it goes in the register command, which at the end looks an appropriate place