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

separate gRPC server create and serve. #9

Closed
wants to merge 1 commit into from

Conversation

0xbillw
Copy link
Contributor

@0xbillw 0xbillw commented Aug 8, 2023

  • fix issue: "grpc: Server.RegisterService after Server.Serve for" as the service register must happen before the Serve() function

  • upgrade some dependencies version

* fix issue: "grpc: Server.RegisterService after Server.Serve for" as the service register must happen before the Serve() function

* upgrade some dependencies version
@0xbillw
Copy link
Contributor Author

0xbillw commented Aug 8, 2023

I made up for the test this time :)

@drgomesp drgomesp self-requested a review August 8, 2023 12:48
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch coverage: 80.00% and project coverage change: -0.60% ⚠️

Comparison is base (ef416e7) 86.95% compared to head (8ef0c63) 86.36%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
- Coverage   86.95%   86.36%   -0.60%     
==========================================
  Files           3        3              
  Lines          46       44       -2     
==========================================
- Hits           40       38       -2     
  Misses          4        4              
  Partials        2        2              
Files Changed Coverage Δ
server.go 81.25% <80.00%> (-2.09%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@drgomesp
Copy link
Owner

drgomesp commented Aug 8, 2023

Thanks for the contribution @0xbillw ,really appreciated!

I forgot I actually had setup some checks for code coverage. If you like to update the code to have a bit more coverage, please go ahead. Otherwise I can tweak the config later today and get this merged as it is.

@0xbillw
Copy link
Contributor Author

0xbillw commented Aug 9, 2023

@drgomesp Sorry, I missed the unit test. I'm very much in favor of more code coverage, and I'll be adding unit tests again to PR.

@0xbillw 0xbillw closed this Aug 10, 2023
@0xbillw 0xbillw deleted the billw/separate_new_and_serve branch August 11, 2023 01:00
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

2 participants