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

Remove Http #198

Closed
sleepdefic1t opened this issue Sep 19, 2020 · 2 comments
Closed

Remove Http #198

sleepdefic1t opened this issue Sep 19, 2020 · 2 comments
Labels
Complexity: High More than 256 lines changed. Difficulty: Intermediate The issue requires basic understanding of the code base. Priority: High The issue or pull request will affect most users. Status: Stale The pull request is in need of updates but there has not been a sufficient response. Type: Breaking Change The issue or pull request documents or introduces a breaking change.

Comments

@sleepdefic1t
Copy link
Contributor

sleepdefic1t commented Sep 19, 2020

Issue(s)

  1. hardcoded http
  2. unique platform requirements
  3. windows cURL still an issue (Setup Windows for GitHub Actions #130)
    Direct Windows solutions:
    • Fix cURL building for Windows
    • Require cURL as a system req
    • Use a pkg mngr (eg Hunter)
      *no pkg mngrs otherwise used in Client/Crypto

Solution

Currently, a call looks like this:

Connection<Api> connection("https://dexplorer.ark.io", 8443);
auto response = connection.api.blockchain.get();

Removing Http:

Connection<Api> connection("https://dexplorer.ark.io", 8443);
auto response = my_custom_get(connection.api.blockchain.get());
// auto whatever = my_custom_method(connections[], requests[]);

Benefits

  1. user decides how to handle resources
  2. doesn’t violate C++ best practices
  3. windows issue now irrelevant
  4. native C++
  5. waaaaaaaaay less code and comp. overhead

Drawbacks

  1. breaking change
  2. users must provide handling
  3. ?

Recommendations

  1. Remove Http internally & update unit tests
  2. Update documentation & examples/tutorials
  3. Target post-v3 APN & Products
@sleepdefic1t sleepdefic1t added Type: Breaking Change The issue or pull request documents or introduces a breaking change. Difficulty: Intermediate The issue requires basic understanding of the code base. Priority: High The issue or pull request will affect most users. Complexity: High More than 256 lines changed. labels Sep 19, 2020
@stale
Copy link

stale bot commented Nov 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Stale The pull request is in need of updates but there has not been a sufficient response. label Nov 13, 2020
@sleepdefic1t
Copy link
Contributor Author

Will resolve in a subsequent PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: High More than 256 lines changed. Difficulty: Intermediate The issue requires basic understanding of the code base. Priority: High The issue or pull request will affect most users. Status: Stale The pull request is in need of updates but there has not been a sufficient response. Type: Breaking Change The issue or pull request documents or introduces a breaking change.
Projects
None yet
Development

No branches or pull requests

1 participant