Skip to content
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

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Document SAP system aggregate #988

merged 3 commits into from
Nov 18, 2022

Conversation

arbulu89
Copy link
Contributor

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

@arbulu89 arbulu89 added the documentation Improvements or additions to documentation label Nov 16, 2022
@arbulu89 arbulu89 force-pushed the document-sap-system-aggregate branch from 18eec36 to c522d2d Compare November 16, 2022 08:58
@arbulu89 arbulu89 marked this pull request as ready for review November 16, 2022 09:07
Copy link
Contributor

@dottorblaster dottorblaster left a 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

@arbulu89
Copy link
Contributor Author

Thanks @dottorblaster ,
All changed. All were good suggestions!

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@fabriziosestito fabriziosestito left a 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!

Copy link
Member

@nelsonkopliku nelsonkopliku left a 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!

Copy link
Member

@EMaksy EMaksy left a 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.

@arbulu89 arbulu89 merged commit a7b5451 into main Nov 18, 2022
@arbulu89 arbulu89 deleted the document-sap-system-aggregate branch November 18, 2022 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Development

Successfully merging this pull request may close these issues.

5 participants