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

fixed declaration dependencies and other lint issues in code base #1743

Merged
merged 1 commit into from Aug 10, 2020
Merged

fixed declaration dependencies and other lint issues in code base #1743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api_versions_request_test.go
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@ package sarama
import "testing"

var (
apiVersionRequest = []byte{}
apiVersionRequest []byte
)

func TestApiVersionsRequest(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions async_producer_test.go
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ import (
"time"

"github.com/fortytw2/leaktest"
metrics "github.com/rcrowley/go-metrics"
"github.com/rcrowley/go-metrics"
)

const TestMessage = "ABC THE MESSAGE"
Expand Down Expand Up @@ -91,7 +91,7 @@ func (f flakyEncoder) Length() int {
}

func (f flakyEncoder) Encode() ([]byte, error) {
if !bool(f) {
if !f {
return nil, errors.New("flaky encoding error")
}
return []byte(TestMessage), nil
Expand Down
4 changes: 2 additions & 2 deletions broker.go
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@ import (
"sync/atomic"
"time"

metrics "github.com/rcrowley/go-metrics"
"github.com/rcrowley/go-metrics"
)

// Broker represents a single Kafka broker connection. All operations on this object are entirely concurrency-safe.
Expand Down Expand Up @@ -1027,7 +1027,7 @@ func (b *Broker) sendAndReceiveV0SASLPlainAuth() error {
length := len(b.conf.Net.SASL.AuthIdentity) + 1 + len(b.conf.Net.SASL.User) + 1 + len(b.conf.Net.SASL.Password)
authBytes := make([]byte, length+4) //4 byte length header + auth data
binary.BigEndian.PutUint32(authBytes, uint32(length))
copy(authBytes[4:], []byte(b.conf.Net.SASL.AuthIdentity+"\x00"+b.conf.Net.SASL.User+"\x00"+b.conf.Net.SASL.Password))
copy(authBytes[4:], b.conf.Net.SASL.AuthIdentity+"\x00"+b.conf.Net.SASL.User+"\x00"+b.conf.Net.SASL.Password)

requestTime := time.Now()
// Will be decremented in updateIncomingCommunicationMetrics (except error)
Expand Down
2 changes: 1 addition & 1 deletion client.go
View file
Open in desktop
Expand Up @@ -788,7 +788,7 @@ func (client *client) backgroundMetadataUpdater() {
}

func (client *client) refreshMetadata() error {
topics := []string{}
var topics []string

if !client.conf.Metadata.Full {
if specificTopics, err := client.MetadataTopics(); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion consumer.go
View file
Open in desktop
Expand Up @@ -623,7 +623,7 @@ func (child *partitionConsumer) parseResponse(response *FetchResponse) ([]*Consu
abortedProducerIDs := make(map[int64]struct{}, len(block.AbortedTransactions))
abortedTransactions := block.getAbortedTransactions()

messages := []*ConsumerMessage{}
var messages []*ConsumerMessage
for _, records := range block.RecordsSet {
switch records.recordsType {
case legacyRecords:
Expand Down
2 changes: 0 additions & 2 deletions examples/README.md
View file
Open in desktop
Expand Up @@ -8,5 +8,3 @@ In these examples, we use `github.com/Shopify/sarama` as import path. We do this

[http_server](./http_server) is a simple HTTP server uses both the sync producer to produce data as part of the request handling cycle, as well as the async producer to maintain an access log. It also uses the [mocks subpackage](https://godoc.org/github.com/Shopify/sarama/mocks) to test both.

#### SASL SCRAM Authentication
[sasl_scram_authentication](./sasl_scram_authentication) is an example of how to authenticate to a Kafka cluster using SASL SCRAM-SHA-256 or SCRAM-SHA-512 mechanisms.
4 changes: 2 additions & 2 deletions examples/sasl_scram_client/main.go
View file
Open in desktop
Expand Up @@ -91,10 +91,10 @@ func main() {
conf.Net.SASL.Handshake = true
if *algorithm == "sha512" {
conf.Net.SASL.SCRAMClientGeneratorFunc = func() sarama.SCRAMClient { return &XDGSCRAMClient{HashGeneratorFcn: SHA512} }
conf.Net.SASL.Mechanism = sarama.SASLMechanism(sarama.SASLTypeSCRAMSHA512)
conf.Net.SASL.Mechanism = sarama.SASLTypeSCRAMSHA512
} else if *algorithm == "sha256" {
conf.Net.SASL.SCRAMClientGeneratorFunc = func() sarama.SCRAMClient { return &XDGSCRAMClient{HashGeneratorFcn: SHA256} }
conf.Net.SASL.Mechanism = sarama.SASLMechanism(sarama.SASLTypeSCRAMSHA256)
conf.Net.SASL.Mechanism = sarama.SASLTypeSCRAMSHA256

} else {
log.Fatalf("invalid SHA algorithm \"%s\": can be either \"sha256\" or \"sha512\"", *algorithm)
Expand Down
12 changes: 6 additions & 6 deletions mockresponses.go
View file
Open in desktop
Expand Up @@ -177,8 +177,8 @@ func (mmr *MockMetadataResponse) For(reqBody versionedDecoder) encoderWithHeader
}

// Generate set of replicas
replicas := []int32{}
offlineReplicas := []int32{}
var replicas []int32
var offlineReplicas []int32
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about this one:
https://play.golang.org/p/vdAdcwi2i09

func main() {
	a := []int32{}
	var b []int32
	
	fmt.Printf("a: %#v\n", a)
	fmt.Printf("b: %#v\n", b)
}

result:
a: []int32{}
b: []int32(nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, yeah, that does changes semantics. but as far as I know, An empty slice can be represented by nil or an empty slice literal. The first approach is preferred as it does not lead to memory allocation. also tests are passing with changing to nil slice declaration. Let me know if that sounds good?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices

Copy link
Contributor

Choose a reason for hiding this comment

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

also tests are passing with changing to nil slice declaration

not sure if we are in a position to trust in the tests 😅

have you tried this branch in a running app?

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked some folks, and they mentioned that before the variable was initialized to an empty array, with your change the array is now nil.

However, append will initialize the array internally

// The append built-in function appends elements to the end of a slice. If
// it has sufficient capacity, the destination is resliced to accommodate the
// new elements. If it does not, a new underlying array will be allocated.

https://play.golang.org/p/4EHaAlO5eIG

so, 🤞 it looks should work

Copy link

Choose a reason for hiding this comment

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

Go makes 'zero-values' do the expected thing. For slices, append, len, cap, range all behave well.

for _, brokerID := range mmr.brokers {
replicas = append(replicas, brokerID)
}
Expand Down Expand Up @@ -772,8 +772,8 @@ func (mr *MockDescribeConfigsResponse) For(reqBody versionedDecoder) encoderWith
Version: req.Version,
}

includeSynonyms := (req.Version > 0)
includeSource := (req.Version > 0)
includeSynonyms := req.Version > 0
includeSource := req.Version > 0

for _, r := range req.Resources {
var configEntries []*ConfigEntry
Expand Down Expand Up @@ -1088,9 +1088,9 @@ func NewMockDescribeLogDirsResponse(t TestReporter) *MockDescribeLogDirsResponse
}

func (m *MockDescribeLogDirsResponse) SetLogDirs(logDirPath string, topicPartitions map[string]int) *MockDescribeLogDirsResponse {
topics := []DescribeLogDirsResponseTopic{}
var topics []DescribeLogDirsResponseTopic
for topic := range topicPartitions {
partitions := []DescribeLogDirsResponsePartition{}
var partitions []DescribeLogDirsResponsePartition
for i := 0; i < topicPartitions[topic]; i++ {
partitions = append(partitions, DescribeLogDirsResponsePartition{
PartitionID: int32(i),
Expand Down
2 changes: 1 addition & 1 deletion tools/kafka-console-producer/kafka-console-producer.go
View file
Open in desktop
Expand Up @@ -102,7 +102,7 @@ func main() {
}

if *headers != "" {
hdrs := []sarama.RecordHeader{}
var hdrs []sarama.RecordHeader
arrHdrs := strings.Split(*headers, ",")
for _, h := range arrHdrs {
if header := strings.Split(h, ":"); len(header) != 2 {
Expand Down
2 changes: 1 addition & 1 deletion tools/kafka-producer-performance/main.go
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@ import (
gosync "sync"
"time"

metrics "github.com/rcrowley/go-metrics"
"github.com/rcrowley/go-metrics"

"github.com/Shopify/sarama"
"github.com/Shopify/sarama/tools/tls"
Expand Down