<p>Thanks <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=301810" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/cbeams">@cbeams</a> for the review!</p>
<p>I will discuss with <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=431064" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/blabno">@blabno</a> about grpc.</p>
<p>As you might be surprised why we created a new branch next to master for the api (as well for the new trade protocol) here is my reasoning behind that:<br>
Keeping those projects outside the scope of Bisq (as it was before) turned out to does not work well. I did not actively follow those projects and the merge policy by the code owner was not following our rules. With the branch (where we apply same strict rules as for master - e.g. need to be production quality) we force ourself (myself mainly) to review and test and get more progress in smaller iterations. I also wanted to avoid that those big projects will get added in one big chunk adding a lot of risk and the difficulty to estimate the BSQ value for it. To break it down in to many PRs over months with requests each month we can stream it better.<br>
For the api there is the problem and risk that people are using it already and it can potentially cause problem for the network and other users if there are bugs. So to get that work reviewed and tested (by me) became more a priority and doing it with a separate branch is the best I could come up. Using master would add much more test efforts as with each release we would need to have much more test scenarios covered. As the branch is "experimental" and not intended to be used by normal user we can relax a bit the requirements for testing compared to the master branch and releases.</p>
<p>Re Kotlin:<br>
I dont' see the dependency defined anywhere. I think it is a transitive dependency from netlayer.<br>
<a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=1070734" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/freimair">@freimair</a> might have a better explaination...</p>
<p>Re bisq.httpapi:<br>
Not 100% sure about http as we have some http package in the p2p module.<br>
So I would prefer api as main package and http as subpackage inside. Thought that might<br>
conflict later if we add grpc, thought to have a grpc as mainpackage would not conflict with expections like with http so to omit there the api might be ok.<br>
Still not optimal as if there is a api module its not clear that the grpc is also an api.<br>
I will discuss with Bernard about grpc and lets postpone that naming issue for a bit later.</p>
<p>Re codeStyles:<br>
Fixed by <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=431064" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/blabno">@blabno</a> and pushed in new commit</p>
<p>Re docs:<br>
Yes a README.md must be added. As well instructions how to run the API.<br>
I for instance could not get it working (although I had it working earlier).<br>
<a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=431064" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/blabno">@blabno</a> can you add those? The doc only need to hanlde the implemented use case (version)<br>
at that moment. Lets add the relevant docs with each feature getting added.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/2253#issuecomment-454026560">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AkpZtiOnh2DbRstWpjvLDun-2QnBU8FPks5vDJaMgaJpZM4Z8YEx">mute the thread</a>.<img src="https://github.com/notifications/beacon/AkpZtniTWI78i0CAKwTTAKkGJBiBz7-eks5vDJaMgaJpZM4Z8YEx.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/bisq-network/bisq","title":"bisq-network/bisq","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/bisq-network/bisq"}},"updates":{"snippets":[{"icon":"PERSON","message":"@ManfredKarrer in #2253: Thanks @cbeams for the review!\r\n\r\nI will discuss with @blabno about grpc. \r\n\r\nAs you might be surprised why we created a new branch next to master for the api (as well for the new trade protocol) here is my reasoning behind that:\r\nKeeping those projects outside the scope of Bisq (as it was before) turned out to does not work well. I did not actively follow those projects and the merge policy by the code owner was not following our rules. With the branch (where we apply same strict rules as for master - e.g. need to be production quality) we force ourself (myself mainly) to review and test and get more progress in smaller iterations. I also wanted to avoid that those big projects will get added in one big chunk adding a lot of risk and the difficulty to estimate the BSQ value for it. To break it down in to many PRs over months with requests each month we can stream it better.\r\nFor the api there is the problem and risk that people are using it already and it can potentially cause problem for the network and other users if there are bugs. So to get that work reviewed and tested (by me) became more a priority and doing it with a separate branch is the best I could come up. Using master would add much more test efforts as with each release we would need to have much more test scenarios covered. As the branch is \"experimental\" and not intended to be used by normal user we can relax a bit the requirements for testing compared to the master branch and releases.\r\n\r\nRe Kotlin:\r\nI dont' see the dependency defined anywhere. I think it is a transitive dependency from netlayer.\r\n@freimair might have a better explaination...\r\n\r\nRe bisq.httpapi:\r\nNot 100% sure about http as we have some http package in the p2p module.\r\nSo I would prefer api as main package and http as subpackage inside. Thought that might \r\nconflict later if we add grpc, thought to have a grpc as mainpackage would not conflict with expections like with http so to omit there the api might be ok.\r\nStill not optimal as if there is a api module its not clear that the grpc is also an api.\r\nI will discuss with Bernard about grpc and lets postpone that naming issue for a bit later.\r\n\r\nRe codeStyles:\r\nFixed by @blabno and pushed in new commit\r\n\r\nRe docs:\r\nYes a README.md must be added. As well instructions how to run the API.\r\nI for instance could not get it working (although I had it working earlier). \r\n@blabno can you add those? The doc only need to hanlde the implemented use case (version) \r\nat that moment. Lets add the relevant docs with each feature getting added.\r\n\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/bisq-network/bisq/pull/2253#issuecomment-454026560"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/2253#issuecomment-454026560",
"url": "https://github.com/bisq-network/bisq/pull/2253#issuecomment-454026560",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>