Skip to content

Commit

Permalink
Remove dependency on "testing" in "thrift" (jaegertracing#586)
Browse files Browse the repository at this point in the history
Signed-off-by: Yuri Shkuro <github@ysh.us>
  • Loading branch information
yurishkuro authored May 24, 2021
1 parent bf267c1 commit ee200a7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 11 deletions.
17 changes: 16 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,30 @@ fmt:
./scripts/updateLicenses.sh

.PHONY: lint
lint:
lint: vet golint lint-fmt lint-thrift-testing

.PHONY: vet
vet:
$(GOVET) $(PACKAGES)

.PHONY: golint
golint:
@cat /dev/null > $(LINT_LOG)
@$(foreach pkg, $(PACKAGES), $(GOLINT) $(pkg) | grep -v crossdock/thrift >> $(LINT_LOG) || true;)
@[ ! -s "$(LINT_LOG)" ] || (echo "Lint Failures" | cat - $(LINT_LOG) && false)

.PHONY: lint-fmt
lint-fmt:
@$(GOFMT) -e -s -l $(ALL_SRC) > $(FMT_LOG)
./scripts/updateLicenses.sh >> $(FMT_LOG)
@[ ! -s "$(FMT_LOG)" ] || (echo "go fmt or license check failures, run 'make fmt'" | cat - $(FMT_LOG) && false)

# make sure thrift/ module does not import "testing"
.PHONY: lint-thrift-testing
lint-thrift-testing:
@cat /dev/null > $(LINT_LOG)
@(grep -rn '"testing"' thrift | grep -v README.md > $(LINT_LOG)) || true
@[ ! -s "$(LINT_LOG)" ] || (echo '"thrift" module must not import "testing", see issue #585' | cat - $(LINT_LOG) && false)

.PHONY: install
install:
Expand Down
2 changes: 2 additions & 0 deletions thrift/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ This is a partial copy of Apache Thrift v0.14.1 (https://github.com/apache/thrif

It is vendored code to avoid compatibility issues with Thrift versions.

The file logger.go is modified to remove dependency on "testing" (see Issuer #585).

See:
* https://github.com/jaegertracing/jaeger-client-go/pull/584
* https://github.com/jaegertracing/jaeger-client-go/pull/303
10 changes: 0 additions & 10 deletions thrift/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package thrift
import (
"log"
"os"
"testing"
)

// Logger is a simple wrapper of a logging function.
Expand Down Expand Up @@ -52,15 +51,6 @@ func StdLogger(logger *log.Logger) Logger {
}
}

// TestLogger is a Logger implementation can be used in test codes.
//
// It fails the test when being called.
func TestLogger(tb testing.TB) Logger {
return func(msg string) {
tb.Errorf("logger called with msg: %q", msg)
}
}

func fallbackLogger(logger Logger) Logger {
if logger == nil {
return StdLogger(nil)
Expand Down

0 comments on commit ee200a7

Please sign in to comment.