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

Add GitHub Actions workflows, use golangci-lint #1573

Merged
merged 5 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* @bai
d1egoaz marked this conversation as resolved.
Show resolved Hide resolved
63 changes: 63 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
name: CI

on: push

jobs:
test:
name: Go ${{ matrix.go-version }} with Kafka ${{ matrix.kafka-version }} on Ubuntu
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
go-version: [1.12.x, 1.13.x]
kafka-version: [2.3.1, 2.4.0]
Copy link
Contributor

Choose a reason for hiding this comment

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

do you think it'd be a good idea also to test against 2.2.x like previously travis did it with 2.2.1?

platform: [ubuntu-latest]

env:
KAFKA_PEERS: localhost:9091,localhost:9092,localhost:9093,localhost:9094,localhost:9095
TOXIPROXY_ADDR: http://localhost:8474
KAFKA_INSTALL_ROOT: /home/runner/kafka
KAFKA_HOSTNAME: localhost
DEBUG: true
KAFKA_VERSION: ${{ matrix.kafka-version }}
KAFKA_SCALA_VERSION: 2.12

steps:
- uses: actions/checkout@v1

- name: Setup Go
uses: actions/setup-go@v1
with:
go-version: ${{ matrix.go-version }}

- uses: actions/cache@v1
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go-

# See https://github.com/actions/setup-go/issues/14
- name: Setup env
run: |
echo "::set-env name=GOPATH::$(go env GOPATH)"
echo "::add-path::$(go env GOPATH)/bin"
shell: bash

- name: Install dependencies
run: |
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.23.1
export REPOSITORY_ROOT=${GITHUB_WORKSPACE}
vagrant/install_cluster.sh
vagrant/boot_cluster.sh
vagrant/create_topics.sh
vagrant/run_java_producer.sh

- name: Run test suite
run: make test

- name: Run linter
run: make lint

- name: Teardown
run: vagrant/halt_cluster.sh
74 changes: 74 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
run:
timeout: 5m
deadline: 10m

linters-settings:
govet:
check-shadowing: false
golint:
min-confidence: 0
gocyclo:
min-complexity: 99
maligned:
suggest-new: true
dupl:
threshold: 100
goconst:
min-len: 2
min-occurrences: 3
misspell:
locale: US
goimports:
local-prefixes: github.com/Shopify/sarama
gocritic:
enabled-tags:
- diagnostic
- experimental
- opinionated
- performance
- style
disabled-checks:
- wrapperFunc
- ifElseChain
funlen:
lines: 300
statements: 300

linters:
disable-all: true
enable:
- bodyclose
# - deadcode
- depguard
- dogsled
# - dupl
# - errcheck
- funlen
# - gocritic
- gocyclo
- gofmt
# - goimports
# - golint
# - gosec
# - gosimple
- govet
# - ineffassign
# - interfacer
# - misspell
# - nakedret
# - scopelint
# - staticcheck
# - structcheck
# - stylecheck
- typecheck
# - unconvert
# - unused
# - varcheck
- whitespace
# - goconst
# - gochecknoinits

issues:
exclude:
- consider giving a name to these results
- include an explanation for nolint directive
9 changes: 4 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
dist: xenial
dist: bionic
language: go
go:
- 1.12.x
Expand All @@ -22,13 +22,12 @@ before_install:
- vagrant/create_topics.sh
- vagrant/run_java_producer.sh

install: make install_dependencies
before_script:
- curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.23.1

script:
- make test
- make vet
- make errcheck
- if [[ "$TRAVIS_GO_VERSION" == 1.13* ]]; then make fmt; fi
- make lint

after_success:
- go tool cover -func coverage.txt
Expand Down
67 changes: 19 additions & 48 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,56 +1,27 @@
export GO111MODULE=on
default: fmt get update test lint

default: fmt vet errcheck test lint
GO := GO111MODULE=on GOPRIVATE=github.com/linkedin GOSUMDB=off go
GOBUILD := CGO_ENABLED=0 $(GO) build $(BUILD_FLAG)
GOTEST := $(GO) test -gcflags='-l' -p 3 -v -race -timeout 6m -coverprofile=profile.out -covermode=atomic

# Taken from https://github.com/codecov/example-go#caveat-multiple-files
.PHONY: test
test:
echo "mode: atomic" > coverage.txt
for d in `go list ./...`; do \
go test -p 1 -v -timeout 6m -race -coverprofile=profile.out -covermode=atomic $$d || exit 1; \
if [ -f profile.out ]; then \
tail +2 profile.out >> coverage.txt; \
rm profile.out; \
fi \
done

GOLINT := $(shell command -v golint)
FILES := $(shell find . -name '*.go' -type f -not -name '*.pb.go' -not -name '*_generated.go' -not -name '*_test.go')
TESTS := $(shell find . -name '*.go' -type f -not -name '*.pb.go' -not -name '*_generated.go' -name '*_test.go')

.PHONY: lint
lint:
ifndef GOLINT
go get golang.org/x/lint/golint
endif
go list ./... | xargs golint

.PHONY: vet
vet:
go vet ./...
get:
$(GO) get ./...
$(GO) mod verify
$(GO) mod tidy

ERRCHECK := $(shell command -v errcheck)
# See https://github.com/kisielk/errcheck/pull/141 for details on ignorepkg
.PHONY: errcheck
errcheck:
ifndef ERRCHECK
go get github.com/kisielk/errcheck
endif
errcheck -ignorepkg fmt github.com/Shopify/sarama/...
update:
$(GO) get -u -v all
$(GO) mod verify
$(GO) mod tidy

.PHONY: fmt
fmt:
@if [ -n "$$(go fmt ./...)" ]; then echo 'Please run go fmt on your code.' && exit 1; fi
gofmt -s -l -w $(FILES) $(TESTS)

.PHONY : install_dependencies
install_dependencies: get

.PHONY: get
get:
go get -v ./...

.PHONY: clean
clean:
go clean ./...
lint:
golangci-lint run

.PHONY: tidy
tidy:
go mod tidy -v
test:
$(GOTEST) ./...
10 changes: 2 additions & 8 deletions Vagrantfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
# -*- mode: ruby -*-
# vi: set ft=ruby :

# Vagrantfile API/syntax version. Don't touch unless you know what you're doing!
VAGRANTFILE_API_VERSION = "2"

# We have 5 * 192MB ZK processes and 5 * 320MB Kafka processes => 2560MB
MEMORY = 3072

Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
config.vm.box = "ubuntu/trusty64"
Vagrant.configure("2") do |config|
config.vm.box = "ubuntu/bionic64"

config.vm.provision :shell, path: "vagrant/provision.sh"

Expand Down
2 changes: 1 addition & 1 deletion add_partitions_to_txn_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestAddPartitionsToTxnRequest(t *testing.T) {
ProducerID: 8000,
ProducerEpoch: 0,
TopicPartitions: map[string][]int32{
"topic": []int32{1},
"topic": {1},
},
}

Expand Down
2 changes: 1 addition & 1 deletion add_partitions_to_txn_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestAddPartitionsToTxnResponse(t *testing.T) {
resp := &AddPartitionsToTxnResponse{
ThrottleTime: 100 * time.Millisecond,
Errors: map[string][]*PartitionError{
"topic": []*PartitionError{&PartitionError{
"topic": {{
Err: ErrInvalidTxnState,
Partition: 2,
}},
Expand Down
6 changes: 0 additions & 6 deletions admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,6 @@ func (ca *clusterAdmin) CreatePartitions(topic string, count int32, assignment [
}

func (ca *clusterAdmin) DeleteRecords(topic string, partitionOffsets map[int32]int64) error {

if topic == "" {
return ErrInvalidTopic
}
Expand Down Expand Up @@ -557,7 +556,6 @@ func (ca *clusterAdmin) DescribeConfig(resource ConfigResource) ([]ConfigEntry,
}

func (ca *clusterAdmin) AlterConfig(resourceType ConfigResourceType, name string, entries map[string]*string, validateOnly bool) error {

var resources []*AlterConfigsResource
resources = append(resources, &AlterConfigsResource{
Type: resourceType,
Expand Down Expand Up @@ -621,7 +619,6 @@ func (ca *clusterAdmin) CreateACL(resource Resource, acl Acl) error {
}

func (ca *clusterAdmin) ListAcls(filter AclFilter) ([]ResourceAcls, error) {

request := &DescribeAclsRequest{AclFilter: filter}

if ca.conf.Version.IsAtLeast(V2_0_0_0) {
Expand Down Expand Up @@ -669,7 +666,6 @@ func (ca *clusterAdmin) DeleteACL(filter AclFilter, validateOnly bool) ([]Matchi
for _, mACL := range fr.MatchingAcls {
mAcls = append(mAcls, *mACL)
}

}
return mAcls, nil
}
Expand All @@ -683,7 +679,6 @@ func (ca *clusterAdmin) DescribeConsumerGroups(groups []string) (result []*Group
return nil, err
}
groupsPerBroker[controller] = append(groupsPerBroker[controller], group)

}

for broker, brokerGroups := range groupsPerBroker {
Expand Down Expand Up @@ -726,7 +721,6 @@ func (ca *clusterAdmin) ListConsumerGroups() (allGroups map[string]string, err e
}

groupMaps <- groups

}(b, ca.conf)
}

Expand Down
6 changes: 1 addition & 5 deletions admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,6 @@ func TestListConsumerGroups(t *testing.T) {
if err != nil {
t.Fatal(err)
}

}

func TestListConsumerGroupsMultiBroker(t *testing.T) {
Expand Down Expand Up @@ -1024,7 +1023,6 @@ func TestListConsumerGroupsMultiBroker(t *testing.T) {
if err != nil {
t.Fatal(err)
}

}

func TestListConsumerGroupOffsets(t *testing.T) {
Expand Down Expand Up @@ -1053,7 +1051,7 @@ func TestListConsumerGroupOffsets(t *testing.T) {
}

response, err := admin.ListConsumerGroupOffsets(group, map[string][]int32{
topic: []int32{0},
topic: {0},
})
if err != nil {
t.Fatalf("ListConsumerGroupOffsets failed with error %v", err)
Expand All @@ -1072,7 +1070,6 @@ func TestListConsumerGroupOffsets(t *testing.T) {
if err != nil {
t.Fatal(err)
}

}

func TestDeleteConsumerGroup(t *testing.T) {
Expand Down Expand Up @@ -1102,7 +1099,6 @@ func TestDeleteConsumerGroup(t *testing.T) {
if err != nil {
t.Fatalf("DeleteConsumerGroup failed with error %v", err)
}

}

// TestRefreshMetaDataWithDifferentController ensures that the cached
Expand Down
6 changes: 3 additions & 3 deletions alter_configs_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestAlterConfigsRequest(t *testing.T) {
configValue := "1000"
request = &AlterConfigsRequest{
Resources: []*AlterConfigsResource{
&AlterConfigsResource{
{
Type: TopicResource,
Name: "foo",
ConfigEntries: map[string]*string{
Expand All @@ -65,14 +65,14 @@ func TestAlterConfigsRequest(t *testing.T) {

request = &AlterConfigsRequest{
Resources: []*AlterConfigsResource{
&AlterConfigsResource{
{
Type: TopicResource,
Name: "foo",
ConfigEntries: map[string]*string{
"segment.ms": &configValue,
},
},
&AlterConfigsResource{
{
Type: TopicResource,
Name: "bar",
ConfigEntries: map[string]*string{
Expand Down
2 changes: 1 addition & 1 deletion alter_configs_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestAlterConfigsResponse(t *testing.T) {

response = &AlterConfigsResponse{
Resources: []*AlterConfigsResourceResponse{
&AlterConfigsResourceResponse{
{
ErrorCode: 0,
ErrorMsg: "",
Type: TopicResource,
Expand Down
Loading