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

Currency Service using C++ #189

Merged
merged 20 commits into from
Jul 11, 2022
Merged

Conversation

DebajitDas
Copy link
Member

@DebajitDas DebajitDas commented Jul 6, 2022

Fixes #36

Re-implemented Currency Service using C++

  1. Added the changelog
  2. Updated the Readme with appropriate steps
  3. Added license header
  4. Incorporated other review comments

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #

@DebajitDas DebajitDas requested a review from a team July 6, 2022 10:19
@cartersocha cartersocha requested a review from lalitb July 6, 2022 18:06
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @DebajitDas!
Thanks for taking care of this one.

I've tested locally and everything is working seamlessly!

Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done. Have few minor review comments.

src/currencyservice/src/client.cpp Show resolved Hide resolved
src/currencyservice/Dockerfile Outdated Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
Copy link
Member

@austinlparker austinlparker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@cartersocha
Copy link
Contributor

@DebajitDas could you update the change log too please?

src/currencyservice/src/client.cpp Show resolved Hide resolved
src/currencyservice/src/server.cpp Outdated Show resolved Hide resolved
@cartersocha cartersocha merged commit 2caa6dc into open-telemetry:main Jul 11, 2022
@DebajitDas DebajitDas deleted the cpp-service branch August 8, 2022 10:57
@reyang reyang mentioned this pull request Aug 16, 2022
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* Saving incremental changes

* Saved the draft

* Added convert function

* Added some more changes

* Updated more changes

* More changes

* Removed js code

* Final changes

* Currency Service in C++

* Cleaned up license

* Removed trailing space

* Incorporated review comments

* Updated sanity failure

* Resolved md error

* Resolved md error

Co-authored-by: Austin Parker <austin@ap2.io>
Co-authored-by: Carter Socha <43380952+cartersocha@users.noreply.github.com>
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.

Reimplement 'currencyservice' in C++
6 participants