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.

Sign up for GitHub

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

Jump to bottom

pkg,cdc: do not use log package #3902

Merged
merged 4 commits into from
Dec 17, 2021
Merged

pkg,cdc: do not use log package #3902

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ff1341
pkg,cdc: do not use log package
overvenus Dec 16, 2021
e279f7f
fix tests
overvenus Dec 16, 2021
0c9601a
address comments
overvenus Dec 17, 2021
4021afb
Merge branch 'master' into no-log
ti-chi-bot Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ linters:
- unconvert
- unparam
- revive
- depguard

linters-settings:
revive:
Expand All @@ -28,3 +29,14 @@ linters-settings:
- name: superfluous-else
- name: modifies-parameter
- name: unreachable-code

depguard:
list-type: blacklist
include-go-root: false
packages:
- log
- github.com/pingcap/errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be github.com/juju/errors?
BTW, seems the lint check CI does not work.

Copy link
Contributor

Choose a reason for hiding this comment

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

/cc @hi-rustin Do you have any idea about the check CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

It works locally.

make check  
check-copyright
gofmt (simplify)
tools/bin/gofumports -l -w $(find . -name '*.go' -type f | grep -vE 'vendor|kv_gen|proto|pb\.go|pb\.gw\.go') 2>&1 | awk '{ print  } END { if (NR > 0) { exit 1  }  }'
run shfmt
tools/bin/shfmt -d -w .
tools/bin/golangci-lint run --timeout 10m0s --skip-files kv_gen --skip-dirs dm
WARN [runner/nolint] Found unknown linters in //nolint directives: exported 
tests/integration_tests/bank/case.go:26:2: `github.com/pingcap/errors` is in the blacklist (depguard)
        "github.com/pingcap/errors"
        ^
tests/mq_protocol_tests/framework/docker_compose_op.go:22:2: `github.com/pingcap/errors` is in the blacklist (depguard)
        "github.com/pingcap/errors"
        ^
tests/mq_protocol_tests/framework/dsl.go:21:2: `github.com/pingcap/errors` is in the blacklist (depguard)
        "github.com/pingcap/errors"
        ^
make: *** [Makefile:210: check-static] Error 1

packages-with-error-message:
# specify an error message to output when a blacklisted package is used
- log: "logging is allowed only by pingcap/log"
- github.com/juju/errors: "error handling is allowed only by pingcap/errors"
3 changes: 2 additions & 1 deletion cdc/puller/frontier/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ package frontier
import (
"bytes"
"fmt"
"log"
"math"
"strings"

_ "unsafe" // required by go:linkname

"github.com/pingcap/log"
)

const (
Expand Down
3 changes: 1 addition & 2 deletions cdc/scheduler/util/table_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
package util

import (
"log"

"github.com/pingcap/log"
"github.com/pingcap/ticdc/cdc/model"
"go.uber.org/zap"
)
Expand Down
2 changes: 1 addition & 1 deletion cdc/sink/codec/canal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ package codec
import (
"context"
"fmt"
"log"
"strconv"
"strings"

"github.com/golang/protobuf/proto" // nolint:staticcheck
"github.com/pingcap/errors"
"github.com/pingcap/log"
"github.com/pingcap/ticdc/cdc/model"
"github.com/pingcap/ticdc/pkg/config"
cerror "github.com/pingcap/ticdc/pkg/errors"
Expand Down
2 changes: 1 addition & 1 deletion cdc/sink/common/flow_control.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
package common

import (
"log"
"sync"
"sync/atomic"

"github.com/edwingeng/deque"
"github.com/pingcap/errors"
"github.com/pingcap/log"
cerrors "github.com/pingcap/ticdc/pkg/errors"
"go.uber.org/zap"
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ package context

import (
"context"
"log"
"time"

"github.com/pingcap/log"
"github.com/pingcap/ticdc/cdc/kv"
"github.com/pingcap/ticdc/cdc/model"
"github.com/pingcap/ticdc/cdc/processor/pipeline/system"
Expand Down
2 changes: 1 addition & 1 deletion pkg/context/context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestThrowPanic(t *testing.T) {
t.Parallel()
defer func() {
panicMsg := recover()
require.Equal(t, panicMsg, "an error has escaped, please report a bug{error 26 0 mock error}")
require.Equal(t, panicMsg, "an error has escaped, please report a bug")
}()
stdCtx := context.Background()
ctx := NewContext(stdCtx, &GlobalVars{})
Expand Down
2 changes: 1 addition & 1 deletion pkg/etcd/etcdkey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
package etcd

import (
"log"
"strings"

"github.com/pingcap/log"
cerror "github.com/pingcap/ticdc/pkg/errors"
)

Expand Down
3 changes: 2 additions & 1 deletion pkg/orchestrator/util/key_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
package util

import (
"log"
"strings"

"github.com/pingcap/log"
)

// EtcdKey represents a complete key in Etcd.
Expand Down
2 changes: 1 addition & 1 deletion tests/mq_protocol_tests/cases/case_date_time.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ package cases

import (
"errors"
"log"
"time"

"github.com/pingcap/log"
"github.com/pingcap/ticdc/tests/mq_protocol_tests/framework"
"github.com/pingcap/ticdc/tests/mq_protocol_tests/framework/avro"
"github.com/pingcap/ticdc/tests/mq_protocol_tests/framework/canal"
Expand Down