Further minor code and comment cleanup

Author: justinclift

common/memcache.go

@@ -5,7 +5,6 @@ import (
 	"crypto/md5"
 	"encoding/gob"
 	"encoding/hex"
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -49,7 +48,7 @@ func ConnectCache() error {
 	cacheTest := memcache.Item{Key: "connecttext", Value: []byte("1"), Expiration: 10}
 	err := memCache.Set(&cacheTest)
 	if err != nil {
-		return errors.New(fmt.Sprintf("Couldn't connect to memcached server: %s", err))
+		return fmt.Errorf("Couldn't connect to memcached server: %s", err)
 	}
 
 	// Log successful connection message for Memcached

common/userinput.go

@@ -1,4 +1,3 @@
-// These functions extract (and validate) user provided form data.
 package common
 
 import (
@@ -114,7 +113,7 @@ func GetFormBranch(r *http.Request) (string, error) {
 	}
 	err = ValidateBranchName(b)
 	if err != nil {
-		return "", errors.New(fmt.Sprintf("Invalid branch name: '%v'", b))
+		return "", fmt.Errorf("Invalid branch name: '%v'", b)
 	}
 	return b, nil
 }

common/util.go

@@ -18,17 +18,16 @@ import (
 )
 
 // AddDatabase is handles database upload processing
-func AddDatabase(r *http.Request, loggedInUser string, dbOwner string, dbFolder string, dbName string,
-	createBranch bool, branchName string, commitID string, public bool, licenceName string, commitMsg string,
-	sourceURL string, newDB io.Reader, serverSw string, lastModified time.Time, commitTime time.Time,
-	authorName string, authorEmail string, committerName string, committerEmail string, otherParents []string,
-	dbSha string) (numBytes int64, newCommitID string, err error) {
+func AddDatabase(r *http.Request, loggedInUser, dbOwner, dbFolder, dbName string, createBranch bool, branchName,
+	commitID string, public bool, licenceName, commitMsg, sourceURL string, newDB io.Reader, serverSw string,
+	lastModified, commitTime time.Time, authorName, authorEmail, committerName, committerEmail string,
+	otherParents []string, dbSha string) (numBytes int64, newCommitID string, err error) {
 
 	// Create a temporary file to store the database in
 	tempDB, err := ioutil.TempFile(Conf.DiskCache.Directory, "dbhub-upload-")
 	if err != nil {
-		log.Printf("Error creating temporary file. User: '%s', Database: '%s%s%s', Filename: '%s', Error: %v\n",
-			loggedInUser, dbOwner, dbFolder, dbName, tempDB.Name(), err)
+		log.Printf("Error creating temporary file. User: '%s', Database: '%s%s%s', Error: %v\n", loggedInUser,
+			dbOwner, dbFolder, dbName, err)
 		return 0, "", err
 	}
 	tempDBName := tempDB.Name()
@@ -401,7 +400,7 @@ func AddDatabase(r *http.Request, loggedInUser string, dbOwner string, dbFolder
 }
 
 // CommitLicenceSHA returns the licence used by the database in a given commit
-func CommitLicenceSHA(dbOwner string, dbFolder string, dbName string, commitID string) (licenceSHA string, err error) {
+func CommitLicenceSHA(dbOwner, dbFolder, dbName, commitID string) (licenceSHA string, err error) {
 	commits, err := GetCommitList(dbOwner, dbFolder, dbName)
 	if err != nil {
 		return "", err
@@ -443,7 +442,7 @@ func CreateDBTreeID(entries []DBTreeEntry) string {
 	for _, j := range entries {
 		b.WriteString(string(j.EntryType))
 		b.WriteByte(0)
-		b.WriteString(string(j.LicenceSHA))
+		b.WriteString(j.LicenceSHA)
 		b.WriteByte(0)
 		b.WriteString(j.Sha256)
 		b.WriteByte(0)
@@ -459,7 +458,7 @@ func CreateDBTreeID(entries []DBTreeEntry) string {
 
 // DeleteBranchHistory safely removes the commit history for a branch, from the head of the branch back to (but not
 // including) the specified commit.  The new branch head will be at the commit ID specified
-func DeleteBranchHistory(dbOwner string, dbFolder string, dbName string, branchName string, commitID string) (isolatedTags []string, isolatedRels []string, err error) {
+func DeleteBranchHistory(dbOwner, dbFolder, dbName, branchName, commitID string) (isolatedTags, isolatedRels []string, err error) {
 	// Make sure the requested commit is in the history for the specified branch
 	ok, err := IsCommitInBranchHistory(dbOwner, dbFolder, dbName, branchName, commitID)
 	if err != nil {
@@ -732,7 +731,7 @@ func DeleteBranchHistory(dbOwner string, dbFolder string, dbName string, branchN
 	}
 
 	// Remove any no-longer-needed commits
-	// TODO: We may want to consider clearing any memcache entries for the deleted commits too
+	// TODO: We may want to consider clearing any Memcache entries for the deleted commits too
 	for cid, del := range checkList {
 		if del == true {
 			delete(commitList, cid)
@@ -745,8 +744,8 @@ func DeleteBranchHistory(dbOwner string, dbFolder string, dbName string, branchN
 // GetCommonAncestorCommits determines the common ancestor commit (if any) between a source and destination branch.
 // Returns the commit ID of the ancestor and a slice of the commits between them.  If no common ancestor exists, the
 // returned ancestorID will be an empty string. Created for use by our Merge Request functions.
-func GetCommonAncestorCommits(srcOwner string, srcFolder string, srcDBName string, srcBranch string, destOwner string,
-	destFolder string, destName string, destBranch string) (ancestorID string, commitList []CommitEntry, err error, errType int) {
+func GetCommonAncestorCommits(srcOwner, srcFolder, srcDBName, srcBranch, destOwner, destFolder, destName,
+	destBranch string) (ancestorID string, commitList []CommitEntry, errType int, err error) {
 
 	// To determine the common ancestor, we retrieve the source and destination commit lists, then starting from the
 	// end of the source list, step backwards looking for a matching ID in the destination list.
@@ -825,7 +824,7 @@ func GetCurrentFunctionName() (FuncName string) {
 }
 
 // IsCommitInBranchHistory checks if a given commit ID is in the history of the given branch
-func IsCommitInBranchHistory(dbOwner string, dbFolder string, dbName string, branchName string, commitID string) (bool, error) {
+func IsCommitInBranchHistory(dbOwner, dbFolder, dbName, branchName, commitID string) (bool, error) {
 	// Get the commit list for the database
 	commitList, err := GetCommitList(dbOwner, dbFolder, dbName)
 	if err != nil {
@@ -934,12 +933,11 @@ func RandomString(length int) string {
 	for i := range randomString {
 		randomString[i] = alphaNum[rand.Intn(len(alphaNum))]
 	}
-
 	return string(randomString)
 }
 
 // StatusUpdateCheck checks if a status update for the user exists for a given discussion or MR, and if so then removes it
-func StatusUpdateCheck(dbOwner string, dbFolder string, dbName string, thisID int, userName string) (numStatusUpdates int, err error) {
+func StatusUpdateCheck(dbOwner, dbFolder, dbName string, thisID int, userName string) (numStatusUpdates int, err error) {
 	var lst map[string][]StatusUpdateEntry
 	lst, err = StatusUpdates(userName)
 	if err != nil {

common/validate.go

@@ -21,10 +21,11 @@ var (
 	regexPGTable         = regexp.MustCompile(`^[a-z,A-Z,0-9,\.,\-,\_,\(,\),\ ]+$`)
 	regexUsername        = regexp.MustCompile(`^[a-z,A-Z,0-9,\.,\-,\_]+$`)
 
-	// For input validation
+	// Validate is used for input validation
 	Validate *valid.Validate
 )
 
+// VisGetFields is used when validating input
 type VisGetFields struct {
 	VisName string `validate:"required,visname,min=1,max=63"` // 63 char limit seems reasonable
 }

webui/main.go

@@ -975,7 +975,7 @@ func createMergeHandler(w http.ResponseWriter, r *http.Request) {
 		SourceOwner:  srcOwner,
 	}
 	var ancestorID string
-	ancestorID, mrDetails.Commits, err, _ = com.GetCommonAncestorCommits(srcOwner, srcFolder, srcDBName, srcBranch,
+	ancestorID, mrDetails.Commits, _, err = com.GetCommonAncestorCommits(srcOwner, srcFolder, srcDBName, srcBranch,
 		destOwner, destFolder, destDBName, destBranch)
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
@@ -2662,7 +2662,7 @@ func diffCommitListHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Get the commit list diff
-	ancestorID, cList, err, errType := com.GetCommonAncestorCommits(srcOwner, srcFolder, srcDBName, srcBranch, destOwner,
+	ancestorID, cList, errType, err := com.GetCommonAncestorCommits(srcOwner, srcFolder, srcDBName, srcBranch, destOwner,
 		destFolder, destDBName, destBranch)
 	if err != nil && errType != http.StatusBadRequest {
 		w.WriteHeader(http.StatusInternalServerError)

webui/pages.go

@@ -598,7 +598,7 @@ func comparePage(w http.ResponseWriter, r *http.Request) {
 
 	// If the initially chosen source and destinations can be directly applied, fill out the initial commit list entries
 	// for display to the user
-	ancestorID, cList, err, errType := com.GetCommonAncestorCommits(dbOwner, dbFolder, dbName,
+	ancestorID, cList, errType, err := com.GetCommonAncestorCommits(dbOwner, dbFolder, dbName,
 		pageData.SourceDBDefaultBranch, pageData.DestOwner, pageData.DestFolder, pageData.DestDBName,
 		pageData.DestDBDefaultBranch)
 	if err != nil && errType != http.StatusBadRequest {
@@ -2537,7 +2537,7 @@ func mergePage(w http.ResponseWriter, r *http.Request) {
 			} else {
 				// Check if the source branch can still be applied to the destination, and also check for new/changed
 				// commits
-				ancestorID, newCommitList, err, _ := com.GetCommonAncestorCommits(mr.MRDetails.SourceOwner,
+				ancestorID, newCommitList, _, err := com.GetCommonAncestorCommits(mr.MRDetails.SourceOwner,
 					mr.MRDetails.SourceFolder, mr.MRDetails.SourceDBName, mr.MRDetails.SourceBranch, dbOwner, dbFolder,
 					dbName, mr.MRDetails.DestBranch)
 				if err != nil {