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

refactor: drop ark-node API (v1) support #34

Merged
merged 4 commits into from Dec 25, 2018
Merged

refactor: drop ark-node API (v1) support #34

merged 4 commits into from Dec 25, 2018

Conversation

sleepdefic1t
Copy link
Contributor

Proposed changes

per #30
covers updates at #32

• Drop V1 code and tests
• Drop "TWO" naming convention
• rename AbstractApi >> API::Abstract
• rename ApiBase >> API::Base
• Update CMake, tests, and docs to reflect changes
• update cmake_example's main.cpp
• Bump version to '1.0.0'

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build (changes that affect the build system)
  • Docs (documentation only changes)
  • Test (adding missing tests or fixing existing tests)
  • Other... Please describe:

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

• Drop V1 code and tests
• Drop "TWO" naming convention
• rename AbstractApi >> API::Abstract
• rename ApiBase >> API::Base
• update cmake_example's main.cpp
• Update CMake, tests, and docs to reflect changes
• Bump version to '1.0.0'
@codecov-io
Copy link

codecov-io commented Dec 14, 2018

Codecov Report

❗ No coverage uploaded for pull request base (develop@94949d7). Click here to learn what that means.
The diff coverage is 98.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop      #34   +/-   ##
==========================================
  Coverage           ?   97.64%           
==========================================
  Files              ?       29           
  Lines              ?      851           
  Branches           ?        0           
==========================================
  Hits               ?      831           
  Misses             ?       20           
  Partials           ?        0
Impacted Files Coverage Δ
src/include/cpp-client/connection/connection.h 100% <ø> (ø)
test/connection/connection.cpp 100% <100%> (ø)
test/api/delegates.cpp 100% <100%> (ø)
src/include/cpp-client/api/peers/peers.h 100% <100%> (ø)
test/api/wallets.cpp 100% <100%> (ø)
test/api/blocks.cpp 100% <100%> (ø)
...include/cpp-client/api/transactions/transactions.h 100% <100%> (ø)
src/include/cpp-client/api/api.h 100% <100%> (ø)
src/api/blocks/blocks.cpp 100% <100%> (ø)
src/api/votes/votes.cpp 100% <100%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94949d7...f92aebc. Read the comment docs.

• Content-Type: application/json
• API-Version: 2
• Accept: application/vnd.ark.core-api.v2+json
@faustbrian faustbrian changed the title refactor: rm-v1-support refactor: drop ark-node API (v1) support Dec 15, 2018
@faustbrian faustbrian changed the base branch from master to develop December 18, 2018 08:22
@faustbrian
Copy link
Contributor

@ciband @sleepdefic1t conflicts

• resolves merge conflicts in ArkEcosystem/Cpp-Client #34
@sleepdefic1t
Copy link
Contributor Author

sleepdefic1t commented Dec 18, 2018

RE: Conflicts

extras/cmake_example/main.cpp

Ark::Client::Connection<Ark::Client::API::TWO> connection("167.114.29.54", 4003); - #35
Ark::Client::Connection<Ark::Client::Api> connection("167.114.29.54", 4003); - Correct (#34)

library.json

Resolved.
Merged changes from #35, version is at 1.0.0.

test/api/two/two_votes.cpp

Should now be test/api/votes.cpp

test/api/votes.cpp ln#155

ASSERT_TRUE(fee == 98500000); - #35
ASSERT_GT(fee, 0); - Correct #34

Tests fail due to pending changes in transactions/types per @supaiku0.
(Involves Typescript enums in Core).

Otherwise, this should be able to be merged.
I don't have write access to resolve conflicts here,
but the PR is now correct, @faustbrian.

@ciband
Copy link
Contributor

ciband commented Dec 18, 2018

You should be able to rebase your branch locally on arkecosystem/cpp-client:master, resolve conflicts locally, commit the merge, then push to this PR branch.

@faustbrian faustbrian merged commit bb1dc19 into ArkEcosystemArchive:develop Dec 25, 2018
@sleepdefic1t sleepdefic1t deleted the refactor/rm-v1-support branch December 27, 2018 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants