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

Implemented tcommit.Manager and tcommit.Resource as gRPC services #242

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

askerka
Copy link

@askerka askerka commented Oct 16, 2021

For #238 - Implemented tcommit.Manager and tcommit.Resource as gRPC services.

Separated service and client packages.

Added marshall package to convert struct between the application and proto definitions.

Updated Finish and Begin client's methods with marshall package

@askerka
Copy link
Author

askerka commented Oct 16, 2021

By the way, the year of the header should be changed 2020 -> 2021

Copy link
Collaborator

@ivanbukhtiyarov ivanbukhtiyarov left a comment

Choose a reason for hiding this comment

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

@askerka Hi! First of all could you check the test logs please? Some tests are failed.

@askerka askerka force-pushed the 238-tcommit-grpc-services branch from 5a28d01 to a04aeb8 Compare October 20, 2021 16:16
@askerka
Copy link
Author

askerka commented Oct 20, 2021

@askerka Hi! First of all could you check the test logs please? Some tests are failed.

@ivanbukhtiyarov Hi! The tests are fixed. Please, have a look

@askerka askerka force-pushed the 238-tcommit-grpc-services branch from a04aeb8 to 441f408 Compare October 20, 2021 20:10
For #238 - Implemented `tcommit.Manager` and `tcommit.Resource` as gRPC services, separated service and client packages, added `marshall` package to convert struct between the application and proto definitions, and updated `Finish` and `Begin` client's methods with `marshall` package
@askerka askerka force-pushed the 238-tcommit-grpc-services branch from 441f408 to 20ce599 Compare October 21, 2021 17:43
@askerka
Copy link
Author

askerka commented Oct 21, 2021

Fixed the code style issues, added nolint:goerr113 because of dynamic errors in tests, and nolint:dupl because tests for commit and abort methods are very similar.

@ivanbukhtiyarov ivanbukhtiyarov self-requested a review October 25, 2021 11:49
Copy link
Collaborator

@ivanbukhtiyarov ivanbukhtiyarov left a comment

Choose a reason for hiding this comment

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

@askerka looks good to me, thanks.

Copy link
Contributor

@g4s8 g4s8 left a comment

Choose a reason for hiding this comment

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

@askerka this PR contains too many changes, which are not related to origin ticket. Could you please keep only relevant #238 changes there?

@askerka
Copy link
Author

askerka commented Oct 28, 2021

@g4s8 There are only two lines that are not relevant:
misspelling and variable construction.

Do you want me to delete them?

Could you please guide me on what changes do you think are not relevant?

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.

3 participants