Skip to content

refactor: remove version argument remains#100

Merged
faustbrian merged 4 commits intomasterfrom
refactor/legacy
Jul 7, 2019
Merged

refactor: remove version argument remains#100
faustbrian merged 4 commits intomasterfrom
refactor/legacy

Conversation

@faustbrian
Copy link
Contributor

Remove all code that related to setting an API-Version as Core 2.5 will only serve the v2 API.

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Refactor
  • Performance
  • Tests
  • Build
  • Documentation
  • Code style update
  • Continuous Integration
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Does this PR release a new version?

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch
  • All tests are passing
  • New/updated tests are included

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

@ghost ghost added Complexity: High More than 256 lines changed. Type: Refactor The pull request improves or enhances an existing implementation. labels Jul 7, 2019
@ghost
Copy link

ghost commented Jul 7, 2019

The ci/circleci: build-macos-9-3 job is failing as of 78f123beed4070c306498865eae66719dedbf966. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Fixes remaining changes from `refactor/legacy`.

Specifically,

- [abstract.h]
  - `explicit` keyword moved to single argument constructor.
  | per: https://google.github.io/styleguide/cppguide.html#Implicit_Conversions
- [http.h]
  - added default constructor;
  - changed copy assignment operator to default.
- [test/mock_api.h]
  - removed legacy version-value from constructor.
- [test/mock_http.h]
  - removed default `MockHTTP()` constructor (now unneeded).
- [test/http.h]
 - fixes to make tests pass with current devnet core.
 - future PR should be done to use some other source.
@codecov-io
Copy link

Codecov Report

Merging #100 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #100   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines         245    242    -3     
=====================================
- Hits          245    242    -3
Impacted Files Coverage Δ
src/http/os/http.cpp 100% <ø> (ø) ⬆️
src/include/cpp-client/api/abstract.h 100% <100%> (ø) ⬆️
src/include/cpp-client/http/http.h 100% <100%> (ø) ⬆️

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 dcea4db...9dca8b0. Read the comment docs.

@faustbrian faustbrian merged commit 8072e50 into master Jul 7, 2019
@ghost ghost deleted the refactor/legacy branch July 7, 2019 16:31
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. Type: Refactor The pull request improves or enhances an existing implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants