Moved to buf; Added buf lint; Fixed ping service to match standards; … #383

bwplotka · 2021-01-17T00:12:22Z

…Moved auth to interceptors.

Chained with #321

Signed-off-by: Bartlomiej Plotka [email protected]

bwplotka · 2021-01-17T11:08:33Z

PTAL @johanbrandhorst @yashrsharma44

codecov-io · 2021-01-17T15:10:08Z

Codecov Report

Merging #383 (e0ea420) into v2 (a79558a) will decrease coverage by 27.48%.
The diff coverage is 37.96%.

@@             Coverage Diff             @@
##               v2     #383       +/-   ##
===========================================
- Coverage   83.58%   56.09%   -27.49%     
===========================================
  Files          30       37        +7     
  Lines         932     1321      +389     
===========================================
- Hits          779      741       -38     
- Misses        114      511      +397     
- Partials       39       69       +30

Impacted Files	Coverage Δ
chain.go	`0.00% <ø> (-90.91%)`	⬇️
interceptors/auth/auth.go	`100.00% <ø> (ø)`
interceptors/auth/metadata.go	`100.00% <ø> (ø)`
interceptors/tags/fieldextractor.go	`13.79% <0.00%> (-71.51%)`	⬇️
testing/testpb/interceptor_suite.go	`0.00% <0.00%> (ø)`
testing/testpb/test.manual_extractfields.pb.go	`0.00% <0.00%> (ø)`
testing/testpb/test.manual_validator.pb.go	`0.00% <0.00%> (ø)`
testing/testpb/test.pb.go	`36.92% <36.92%> (ø)`
interceptors/logging/payload.go	`67.18% <42.30%> (-15.43%)`	⬇️
testing/testpb/test_grpc.pb.go	`45.55% <45.55%> (ø)`
... and 45 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 c1f1e53...e0ea420. Read the comment docs.

johanbrandhorst · 2021-01-18T11:46:54Z

Makefile

+proto: ## Generate testing protobufs
+proto: $(BUF) $(PROTOC_GEN_GO) $(PROTOC_GEN_GO_GRPC) $(PROTO_TEST_DIR)/test.proto


Should we have two targets name proto? I'm not familiar with this Makefile pattern, if it is one.

I can see this being confusing, but it's totally ok. It's to ensure make help works (and shows this command)

johanbrandhorst · 2021-01-18T11:47:35Z

Makefile

+	@PATH=$(GOBIN):$(TMP_GOPATH) $(BUF) protoc \
+		-I $(PROTO_TEST_DIR) \
+		--go_out=$(PROTO_TEST_DIR)/../ \
+		--go-grpc_out=$(PROTO_TEST_DIR)/../ \
+	    $(PROTO_TEST_DIR)/*.proto


This is awesome, next step would be to just use buf generate: https://docs.buf.build/tour-6/. If you're feeling keen it should be possible to do here already with buf.gen.yaml:

version: v1beta1 plugins: - name: go out: testing/testpb/v1 - name: go-grpc out: testing/testpb/v1

You may want opt: paths=source_relative if the files aren't going in the right place.

uuu nice, let's try it (or in sep PR)

yashrsharma44

Looks good! 😎

yashrsharma44 · 2021-01-18T14:52:34Z

testing/testpb/v1/test.proto

@@ -0,0 +1,68 @@
+syntax = "proto3";
+
+package testing.testpb.v1;


Looks solid 💪

yashrsharma44 · 2021-01-18T14:54:39Z

.bingo/Variables.mk

@@ -23,6 +23,12 @@ $(BINGO): $(BINGO_DIR)/bingo.mod
 	@echo "(re)installing $(GOBIN)/bingo-v0.3.0"
 	@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=bingo.mod -o=$(GOBIN)/bingo-v0.3.0 "github.com/bwplotka/bingo"

+BUF := $(GOBIN)/buf-v0.35.1


Why are we adding buf dependency? Wanted to know for info purpose 😛

Because we use now buf as a tool! (:

bwplotka · 2021-01-18T19:11:21Z

Look like this is ok to be merged (:

Let's make sure we merge after #321 is approved

…Moved auth to interceptors. Signed-off-by: Bartlomiej Plotka <[email protected]>

bwplotka · 2021-01-22T14:36:04Z

Rebased, rdy to merge (:

Signed-off-by: Bartlomiej Plotka <[email protected]>

bwplotka requested review from yashrsharma44 and johanbrandhorst January 17, 2021 00:12

bwplotka force-pushed the update-proto-yash branch 3 times, most recently from 3c95098 to a4f5db6 Compare January 17, 2021 10:26

bwplotka force-pushed the buf branch from 22dadf1 to 37e8fdb Compare January 17, 2021 10:32

bwplotka force-pushed the update-proto-yash branch 2 times, most recently from e83983b to 49fc59c Compare January 17, 2021 10:59

bwplotka force-pushed the buf branch from 37e8fdb to 45b28c0 Compare January 17, 2021 11:07

bwplotka force-pushed the buf branch 3 times, most recently from 7ff2f54 to f8379a9 Compare January 17, 2021 15:08

johanbrandhorst approved these changes Jan 18, 2021

View reviewed changes

yashrsharma44 approved these changes Jan 18, 2021

View reviewed changes

testing/testpb/v1/test.proto

@@ -0,0 +1,68 @@

syntax = "proto3";

package testing.testpb.v1;

Copy link

Collaborator

yashrsharma44 Jan 18, 2021

Choose a reason for hiding this comment

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

Looks solid 💪

yashrsharma44 reviewed Jan 18, 2021

View reviewed changes

bwplotka force-pushed the update-proto-yash branch from 55f20ac to 582d3a5 Compare January 20, 2021 17:06

Base automatically changed from update-proto-yash to v2 January 22, 2021 14:30

Moved to buf; Added buf lint; Fixed ping service to match standards; …

c03359b

…Moved auth to interceptors. Signed-off-by: Bartlomiej Plotka <[email protected]>

bwplotka force-pushed the buf branch from f8379a9 to c03359b Compare January 22, 2021 14:35

Fixed flakiness.

e0ea420

Signed-off-by: Bartlomiej Plotka <[email protected]>

bwplotka merged commit 468f615 into v2 Jan 22, 2021

bwplotka deleted the buf branch January 22, 2021 20:43

aimuz mentioned this pull request Mar 8, 2023

docs: add logging.InjectFields usage description #540

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved to buf; Added buf lint; Fixed ping service to match standards; … #383

Moved to buf; Added buf lint; Fixed ping service to match standards; … #383

bwplotka commented Jan 17, 2021

bwplotka commented Jan 17, 2021

codecov-io commented Jan 17, 2021 •

edited

Loading

johanbrandhorst Jan 18, 2021

bwplotka Jan 18, 2021

johanbrandhorst Jan 18, 2021

bwplotka Jan 18, 2021

yashrsharma44 left a comment

yashrsharma44 Jan 18, 2021

yashrsharma44 Jan 18, 2021

bwplotka Jan 18, 2021

bwplotka commented Jan 18, 2021

bwplotka commented Jan 22, 2021

		proto: ## Generate testing protobufs
		proto: $(BUF) $(PROTOC_GEN_GO) $(PROTOC_GEN_GO_GRPC) $(PROTO_TEST_DIR)/test.proto

		@@ -0,0 +1,68 @@
		syntax = "proto3";

		package testing.testpb.v1;

Moved to buf; Added buf lint; Fixed ping service to match standards; … #383

Moved to buf; Added buf lint; Fixed ping service to match standards; … #383

Conversation

bwplotka commented Jan 17, 2021

bwplotka commented Jan 17, 2021

codecov-io commented Jan 17, 2021 • edited Loading

Codecov Report

johanbrandhorst Jan 18, 2021

Choose a reason for hiding this comment

bwplotka Jan 18, 2021

Choose a reason for hiding this comment

johanbrandhorst Jan 18, 2021

Choose a reason for hiding this comment

bwplotka Jan 18, 2021

Choose a reason for hiding this comment

yashrsharma44 left a comment

Choose a reason for hiding this comment

yashrsharma44 Jan 18, 2021

Choose a reason for hiding this comment

yashrsharma44 Jan 18, 2021

Choose a reason for hiding this comment

bwplotka Jan 18, 2021

Choose a reason for hiding this comment

bwplotka commented Jan 18, 2021

bwplotka commented Jan 22, 2021

codecov-io commented Jan 17, 2021 •

edited

Loading