changes suggested by golint

Author: Brian Picciano

clients/clients.go

@@ -18,26 +18,27 @@ func init() {
 	}()
 }
 
-// Obstensibly a net.Conn, but for testing we don't want to have to set up a
-// real listen socket and all that noise
+// ClientConn is obstensibly a net.Conn, but for testing we don't want to have
+// to set up a real listen socket and all that noise
 type ClientConn interface {
 	io.ReadWriteCloser
 	RemoteAddr() net.Addr
 }
 
-// Represents a single client, either one just submitting events or a consumer.
-// It expects to be handled in a single threaded context, except for methods
-// marked otherwise (specifically Notify and DrainNotifyCh)
+// Client represents a single okq client, either one just submitting events or a
+// consumer. It expects to be handled in a single threaded context, except for
+// methods marked otherwise (specifically Notify and DrainNotifyCh)
 type Client struct {
-	Id       string
+	ID       string
 	Queues   []string
 	Conn     ClientConn
 	NotifyCh chan string
 }
 
+// NewClient creates a new Client structure around the given ClientConn
 func NewClient(conn ClientConn) *Client {
 	client := &Client{
-		Id:       <-uuidCh,
+		ID:       <-uuidCh,
 		Conn:     conn,
 		Queues:   []string{},
 		NotifyCh: make(chan string, 1),
@@ -46,34 +47,36 @@ func NewClient(conn ClientConn) *Client {
 	return client
 }
 
-// Notifies the client that queueName has an event on it. This may be called
-// from another thread besides the one which "owns" the client
+// Notify notifies the client that queueName has an event on it. This may be
+// called from another thread besides the one which "owns" the client
 func (client *Client) Notify(queueName string) {
 	select {
 	case client.NotifyCh <- queueName:
 	default:
 	}
 }
 
-// Removes any queue notifications that may be buffered in the client. This may
-// be called from another thread besides the one which "owns" the client
+// DrainNotifyCh removes any queue notifications that may be buffered in the
+// client. This may be called from another thread besides the one which "owns"
+// the client
 func (client *Client) DrainNotifyCh() {
 	select {
 	case <-client.NotifyCh:
 	default:
 	}
 }
 
+// Close closes the client's connection
 func (client *Client) Close() {
 	client.Conn.Close()
 }
 
-// Returns the given formatted string with a little extra info about the client
-// prepended to it
+// Sprintf returns the given formatted string with a little extra info about the
+// client prepended to it
 func (client *Client) Sprintf(format string, args ...interface{}) error {
 	fullFormat := "client %v %v - " + format
 	fullArgs := make([]interface{}, 0, len(args)+2)
-	fullArgs = append(fullArgs, client.Id)
+	fullArgs = append(fullArgs, client.ID)
 	fullArgs = append(fullArgs, client.Conn.RemoteAddr())
 	fullArgs = append(fullArgs, args...)
 

clients/consumers/active.go

@@ -16,17 +16,20 @@ import (
 //
 // * Update the timestamps of all the currently live consumers on this instance
 //   in redis, so other okq instances don't truncate them
+
 const (
-	STALE_CONSUMER_TIMEOUT = 30 * time.Second
+	// StaleConsumerTimeout is the time a consumer has to update its register
+	// status on a queue before it is considered stale and removed
+	StaleConsumerTimeout = 30 * time.Second
 )
 
 func activeSpin() {
 	tick := time.Tick(10 * time.Second)
-	for _ = range tick {
+	for range tick {
 		if err := updateActiveConsumers(); err != nil {
 			log.L.Printf("updating active consumers: %s", err)
 		}
-		if err := removeStaleConsumers(STALE_CONSUMER_TIMEOUT); err != nil {
+		if err := removeStaleConsumers(StaleConsumerTimeout); err != nil {
 			log.L.Printf("removing stale consumers: %s", err)
 		}
 	}
@@ -50,7 +53,7 @@ func updateActiveConsumers() error {
 				consumersArgs[queue] = args
 			}
 
-			consumersArgs[queue] = append(args, ts, client.Id)
+			consumersArgs[queue] = append(args, ts, client.ID)
 		}
 	}
 

clients/consumers/consumers.go

@@ -33,11 +33,11 @@ type state struct {
 
 // Returns a slice containing a client's current queues, or empty slice if there
 // are none registered
-func (s *state) clientQueues(clientId string) []string {
+func (s *state) clientQueues(clientID string) []string {
 	queues := make([]string, 0, 8)
 	for q, clients := range s.queueM {
 		for _, client := range clients {
-			if client.Id == clientId {
+			if client.ID == clientID {
 				queues = append(queues, q)
 			}
 		}
@@ -50,9 +50,9 @@ func (s *state) clientQueues(clientId string) []string {
 func (s *state) addClientQueues(client *clients.Client, queues []string) {
 	for _, q := range queues {
 		if mc := s.queueM[q]; mc != nil {
-			mc[client.Id] = client
+			mc[client.ID] = client
 		} else {
-			s.queueM[q] = map[string]*clients.Client{client.Id: client}
+			s.queueM[q] = map[string]*clients.Client{client.ID: client}
 		}
 	}
 }
@@ -61,7 +61,7 @@ func (s *state) addClientQueues(client *clients.Client, queues []string) {
 func (s *state) removeClientQueues(client *clients.Client, queues []string) {
 	for _, q := range queues {
 		if mc := s.queueM[q]; mc != nil {
-			delete(mc, client.Id)
+			delete(mc, client.ID)
 			if len(mc) == 0 {
 				delete(s.queueM, q)
 			}
@@ -72,33 +72,33 @@ func (s *state) removeClientQueues(client *clients.Client, queues []string) {
 // Returns a slice containing a queue's currently registered clients, or empty
 // slice if there are none registered
 func (s *state) queueClients(queue string) []*clients.Client {
-	if mc := s.queueM[queue]; mc == nil {
+	mc := s.queueM[queue]
+	if mc == nil {
 		return []*clients.Client{}
-	} else {
-		cs := make([]*clients.Client, 0, len(mc))
-		for _, c := range mc {
-			cs = append(cs, c)
-		}
-		return cs
 	}
+	cs := make([]*clients.Client, 0, len(mc))
+	for _, c := range mc {
+		cs = append(cs, c)
+	}
+	return cs
 }
 
 // Returns a slice of all queues currently registered by at least one client
 func (s *state) registeredQueues() []string {
 	qs := make([]string, 0, len(s.queueM))
-	for q, _ := range s.queueM {
+	for q := range s.queueM {
 		qs = append(qs, q)
 	}
 	return qs
 }
 
-// Should be called whenever a client changes what queues it's registered for.
-// The new full list of registered queues should be passed in, this method will
-// do a diff and figure it out what was removed
+// UpdateQueues should be called whenever a client changes what queues it's
+// registered for. The new full list of registered queues should be passed in,
+// this method will do a diff and figure it out what was removed
 func UpdateQueues(client *clients.Client, queues []string) error {
 	respCh := make(chan error)
 	callCh <- func(s *state) {
-		oldQueues := s.clientQueues(client.Id)
+		oldQueues := s.clientQueues(client.ID)
 		removed := stringSliceSub(oldQueues, queues)
 		s.addClientQueues(client, queues)
 		s.removeClientQueues(client, removed)
@@ -120,12 +120,12 @@ func UpdateQueues(client *clients.Client, queues []string) error {
 
 		for _, queueName := range removed {
 			consumersKey := db.ConsumersKey(queueName)
-			redisClient.Append("ZREM", consumersKey, client.Id)
+			redisClient.Append("ZREM", consumersKey, client.ID)
 			pipelineSize++
 		}
 		for _, queueName := range queues {
 			consumersKey := db.ConsumersKey(queueName)
-			redisClient.Append("ZADD", consumersKey, ts, client.Id)
+			redisClient.Append("ZADD", consumersKey, ts, client.ID)
 			pipelineSize++
 		}
 		for i := 0; i < pipelineSize; i++ {
@@ -173,8 +173,8 @@ outer:
 	return ret
 }
 
-// Returns the total number of consumers registered for the given queue, either
-// on this okq instance or others
+// QueueConsumerCount returns the total number of consumers registered for the
+// given queue, either on this okq instance or others
 func QueueConsumerCount(queue string) (int64, error) {
 	consumersKey := db.ConsumersKey(queue)
 

clients/consumers/consumers_test.go

@@ -29,7 +29,7 @@ func TestUpdateQueues(t *T) {
 	// Make sure the client.Id appears in the consumers set for those queues
 	for i := range queues {
 		key := db.ConsumersKey(queues[i])
-		res := redisClient.Cmd("ZRANK", key, client.Id)
+		res := redisClient.Cmd("ZRANK", key, client.ID)
 		assert.Equal(t, redis.IntegerReply, res.Type, "res: %s", res)
 	}
 
@@ -38,13 +38,13 @@ func TestUpdateQueues(t *T) {
 
 	// Make sure the first queue had this clientId removed from it
 	key := db.ConsumersKey(queues[0])
-	res := redisClient.Cmd("ZRANK", key, client.Id)
+	res := redisClient.Cmd("ZRANK", key, client.ID)
 	assert.Equal(t, redis.NilReply, res.Type, "res: %s", res)
 
 	// Make sure the rest of the queues still have it
 	for i := range queues[1:] {
 		key := db.ConsumersKey(queues[1:][i])
-		res := redisClient.Cmd("ZRANK", key, client.Id)
+		res := redisClient.Cmd("ZRANK", key, client.ID)
 		assert.Equal(t, redis.IntegerReply, res.Type, "res: %s", res)
 	}
 
@@ -54,7 +54,7 @@ func TestUpdateQueues(t *T) {
 	// Make sure the clientId appears nowhere
 	for i := range queues {
 		key := db.ConsumersKey(queues[i])
-		res := redisClient.Cmd("ZRANK", key, client.Id)
+		res := redisClient.Cmd("ZRANK", key, client.ID)
 		assert.Equal(t, redis.NilReply, res.Type, "res: %s", res)
 	}
 
@@ -73,7 +73,7 @@ func TestStaleCleanup(t *T) {
 
 	// Make sure the queue has this clientId as a consumer
 	key := db.ConsumersKey(queue)
-	res := redisClient.Cmd("ZRANK", key, client.Id)
+	res := redisClient.Cmd("ZRANK", key, client.ID)
 	assert.Equal(t, redis.IntegerReply, res.Type, "res: %s", res)
 
 	// Remove all knowledge about this client from the consumer state
@@ -87,7 +87,7 @@ func TestStaleCleanup(t *T) {
 	require.Nil(t, err)
 
 	// Make sure this client is no longer a consumer
-	res = redisClient.Cmd("ZRANK", key, client.Id)
-	assert.Equal(t, redis.NilReply, res.Type, "key: %s clientId: %s res: %s", key, client.Id, res)
+	res = redisClient.Cmd("ZRANK", key, client.ID)
+	assert.Equal(t, redis.NilReply, res.Type, "key: %s clientId: %s res: %s", key, client.ID, res)
 
 }

clients/fake_client.go

@@ -7,29 +7,36 @@ import (
 	"net"
 )
 
-// Implements clients.ClientConn, but isn't an actual network connection. Can
-// still be passed into resp.ReadMessage and other things that take in
+// FakeClientConn implements ClientConn, but isn't an actual network connection.
+// Can still be passed into resp.ReadMessage and other things that take in
 // io.Readers and io.Writers. Used for testing here and other places
 type FakeClientConn struct {
 	*bytes.Buffer
 }
 
+// NewFakeClientConn initializes a FakeClientConn and returns it
 func NewFakeClientConn() *FakeClientConn {
 	return &FakeClientConn{
 		bytes.NewBuffer(make([]byte, 0, 1024)),
 	}
 }
 
+// Close does nothing and always returns nil. It is only here to implement the
+// ClientConn interface
 func (fconn *FakeClientConn) Close() error {
 	return nil
 }
 
+// RemoteAddr always returns nil. It is only here to implement the ClientConn
+// interface
 func (fconn *FakeClientConn) RemoteAddr() net.Addr {
 	return nil
 }
 
-// Kind of an odd place for this, but it has to go somewhere. Returns a random
-// string to use as a queue name. Used for testing in a few places
+// Kind of an odd place for this, but it has to go somewhere.
+
+// RandQueueName returns a random string to use as a queue name. Used for
+// testing in a few places
 func RandQueueName() string {
 	b := make([]byte, 10)
 	if _, err := rand.Read(b); err != nil {

commands/commands.go

@@ -1,4 +1,4 @@
-// Contains all commands callable by a client
+// Package commands contains all commands callable by a client
 package commands
 
 import (
@@ -40,10 +40,9 @@ var commandMap = map[string]commandInfo{
 
 var okSS = resp.NewSimpleString("OK")
 
-// All commands take in a client whose command has already been read off the
+// Dispatch takes in a client whose command has already been read off the
 // socket, a list of arguments from that command (not including the command name
-// itself), and return an error ONLY if the error is worth logging (disconnect
-// from redis, etc...)
+// itself), and handles that command
 func Dispatch(client *clients.Client, cmd string, args []string) {
 	cmdInfo, ok := commandMap[strings.ToUpper(cmd)]
 	if !ok {

commands/commands_test.go

@@ -81,22 +81,22 @@ func TestQRegister(t *T) {
 func TestBasicFunctionality(t *T) {
 	client := newClient()
 	queue := clients.RandQueueName()
-	events := []struct{ eventId, event string }{
+	events := []struct{ eventID, event string }{
 		{"0", "foo"},
 		{"1", "bar"},
 		{"2", "baz"},
 	}
 
 	for i := range events {
-		Dispatch(client, "qlpush", []string{queue, events[i].eventId, events[i].event})
+		Dispatch(client, "qlpush", []string{queue, events[i].eventID, events[i].event})
 		readAndAssertStr(t, client, "OK")
 	}
 
 	for i := range events {
 		Dispatch(client, "qrpop", []string{queue})
-		readAndAssertArr(t, client, []string{events[i].eventId, events[i].event})
+		readAndAssertArr(t, client, []string{events[i].eventID, events[i].event})
 
-		Dispatch(client, "qack", []string{queue, events[i].eventId})
+		Dispatch(client, "qack", []string{queue, events[i].eventID})
 		readAndAssertInt(t, client, 1)
 	}
 }
@@ -118,7 +118,7 @@ func TestQStatus(t *T) {
 func TestPeeks(t *T) {
 	client := newClient()
 	queue := clients.RandQueueName()
-	events := []struct{ eventId, event string }{
+	events := []struct{ eventID, event string }{
 		{"0", "foo"},
 		{"1", "bar"},
 		{"2", "baz"},
@@ -133,15 +133,15 @@ func TestPeeks(t *T) {
 	readAndAssertNil(t, client)
 
 	for i := range events {
-		Dispatch(client, "qlpush", []string{queue, events[i].eventId, events[i].event})
+		Dispatch(client, "qlpush", []string{queue, events[i].eventID, events[i].event})
 		readAndAssertStr(t, client, "OK")
 	}
 
 	Dispatch(client, "qrpeek", []string{queue})
-	readAndAssertArr(t, client, []string{eventFirst.eventId, eventFirst.event})
+	readAndAssertArr(t, client, []string{eventFirst.eventID, eventFirst.event})
 
 	Dispatch(client, "qlpeek", []string{queue})
-	readAndAssertArr(t, client, []string{eventLast.eventId, eventLast.event})
+	readAndAssertArr(t, client, []string{eventLast.eventID, eventLast.event})
 
 	// Make sure the actual status of the queue hasn't been affected
 	Dispatch(client, "qstatus", []string{queue})