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

For Cycle 16 #642

Closed
ghubstan opened this issue Aug 20, 2020 · 8 comments
Closed

For Cycle 16 #642

ghubstan opened this issue Aug 20, 2020 · 8 comments
Assignees
Labels
parsed:valid https://bisq.wiki/Compensation#Ensure_your_request_is_valid team:dev https://bisq.wiki/Dev_Team was:accepted Indicates that a compensation request was accepted by DAO voting
Milestone

Comments

@ghubstan
Copy link

ghubstan commented Aug 20, 2020

Summary

Specify the total amount of BSQ you are requesting, along with the USD total and BSQ/USD rate (don't include the brackets!):

  • BSQ requested: 14706
  • USD requested: 10000
  • BSQ rate: 0.68 USD per BSQ
  • Previous compensation request (if applicable): For Cycle 15 #620

Contributions delivered

Add contributions you have delivered and roles you have performed here as new rows in the table below. Role line-items should include an asterisk (*) in the team column.

Title Team USD Link Notes
Add API test harness for Linux/OSX dev 10000 bisq-network/bisq#4366 merged
Move gRPC boilerplate from :core to :daemon dev 0 https://github.com/bisq-network/bisq/pull/44127 merged
Remove duplicate CompositeOptionSet class dev 0 https://github.com/bisq-network/bisq/pull/44128 merged

Contributions in progress

Provide links to work you're involved with that is still in progress. This section is optional (the linter ignores it), and is for your own benefit in keeping track of what you're doing and keeping other contributors up to date with the same.

Title Team USD Link Notes
Fix setUpScaffold() signature dev 0 bisq-network/bisq#4429 merged
Move bats based mainnet test from :cli to :apitest dev 0 bisq-network/bisq#4430 merged
Remove duplicated grpc stub creation logic dev 0 bisq-network/bisq#4431 merged
Remove redundant ApiTestConfig options dev 0 bisq-network/bisq#4432 merged
Allow remote debugging of background Bisq apps dev 0 bisq-network/bisq#4433 merged
Make CoreApi a singleton dev --- bisq-network/bisq#4461 merged
Add core support for registration of regtest dispute agents via API dev --- bisq-network/bisq#4524 merged
Refactor API & add registerdisputeagent method to CLI dev --- bisq-network/bisq#4539 awaiting review
Refactor apitest for api calls to any daemon type dev --- bisq-network/bisq#4525 merged
Give core api a simple way to verify init status dev --- bisq-network/bisq#4534 awaiting review
Reduce apitest harness' dependency on string matching dev --- bisq-network/bisq#4540 awaiting review
@ghubstan ghubstan changed the title [WIP] For Cycle 16 For Cycle 16 Aug 20, 2020
@ghost ghost added parsed:valid https://bisq.wiki/Compensation#Ensure_your_request_is_valid team:dev https://bisq.wiki/Dev_Team labels Aug 20, 2020
@MwithM MwithM added this to the Cycle 16 milestone Aug 21, 2020
@sqrrm
Copy link
Member

sqrrm commented Aug 23, 2020

I have a hard time valuing this work. While we do need a test framework and I think the code is good, there is a lot of money going into code that's not actually useful for end users.

On the other hand, we have previously been severely lacking in tests, and this framework will allow us to set up more live like tests. Very good stuff, but useless if Bisq is no longer around.

I suggest that we should focus on producing features for api users going forward, especially now that we have this test framework. There will likely be traders that are happy to start tinkering at that point and we will quickly learn where we need to focus.

@ripcurlx
Copy link
Contributor

Sorry for jumping in that late because of vacation. I agree what @sqrrm is mentioning above. I'll have a look if the Codacy issues are wrong so it can be merged to master and compensated.

@ghubstan
Copy link
Author

@sqrrm @ripcurlx:

I have a hard time valuing this work...

I expected this valid questioning and pushback to the CR. I can be flexible about the amount requested, but when I was ready to submit the PR about a month ago, I thought it was worth more than 10K USD.

Let me respond to your comments and give you my plan for the next phase of development. Maybe it will help you decide to approve this CR as is, or not.

Although the apitest harness gives no benefits to Bisq users today, it can provide benefits to other code contributors now, and the test harness will benefit the ongoing work on the API, which I will talk about in a moment.

Below is a list of tasks I am currently working on, starting with the work already done, and saved in a scratch branch.

Finish and fix apitest functionality (already done, to go into new PRs but not to be compensated):

  • Fix test case setup method to accept any combination of options
  • Move :cli/test.sh to :apitest/scripts/mainnet-test.sh
  • Remove duplicated gRPC stub creation logic
  • Remove 3 redundant config options
  • Add option to run background Bisq apps with remote debug options

Below are descriptions of two completed tasks I will add to new PRs, and be part of my next CR (Cycle 17):

  1. I had to upgrade JFoenix to v9.0.10 in my scratch branch, to avoid an NPE being thrown when I try to register a dispute agent in an arb desktop running in regtest/dao mode. I came across this JFoenix lib bug while using apitest to figure out how get the API to register dispute and refund agents -- required for trading with the API.

  2. I completed one task directly related to the API, a new ping method to be used for checking server availability before any request.

The next task is figure out how, then implement registration of dispute and refund agents in an arbitration :daemon.

After that, I go back to the "simplest trading script" task list defined in Issue 4257, and implmenent creatoffer, confirmoffer, takeoffer, confirmtakeoffer, getofferstatus, paymentstarted, paymentreceived, canceloffer, movefunds, and withdrawfunds methods and test cases.

Now that apitest is out of the way, I can focus on the "simple api trading" script work again. I do think I have a good chance of implementing all or most of these new methods in the next 3-4 weeks (if apitest is truly "out of the way"). This is when the benefits of apitest will become more tangible.

@ghubstan
Copy link
Author

The apitest related scratch branch commits I mentioned above are now PRs, and listed in the Contributions in progress section of this CR.

@sqrrm
Copy link
Member

sqrrm commented Aug 26, 2020

Having considered this a bit further, I approve of this compensation request seeing it as a part of the whole API project. It's already able to run the tests which is something we haven't been able to do before. It will also help speed up the upcoming API dev.

@ghubstan
Copy link
Author

Proposal tx: 0128c8cb88438563afea73b5cb5fbc77eff6eb87be16634f87efb0eb94d6c112

@MwithM MwithM added the was:accepted Indicates that a compensation request was accepted by DAO voting label Sep 4, 2020
@ghost
Copy link

ghost commented Sep 4, 2020

Issuance by Team:

team amount BSQ amount USD
dev 14706.00 10000.00

Total Issuance: 14706.00 BSQ (equivalent to: 10000.00 USD)

@MwithM
Copy link
Contributor

MwithM commented Sep 4, 2020

was:accepted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parsed:valid https://bisq.wiki/Compensation#Ensure_your_request_is_valid team:dev https://bisq.wiki/Dev_Team was:accepted Indicates that a compensation request was accepted by DAO voting
Projects
Archived in project
Development

No branches or pull requests

4 participants