refactor: add lint test and refactor code.

- lint test using golangci-lint.
- lint test added as a job in the testing stage.
- code refactoring based on feedback.

.gitlab-ci.yml

@@ -6,18 +6,28 @@ stages:
 variables:
   CGO_ENABLED: 0
 
+.install-make:
+  before_script:
+  - apk add --no-cache make
+
 test:unit:
   artifacts:
     expire_in: 30 minutes
     paths:
     - code-coverage.html
-  before_script:
-  - apk add --no-cache make
+  extends: .install-make
   image: golang:1.13.6-alpine
   script:
   - make test_cover_report
   stage: test
 
+test:lint:
+  extends: .install-make
+  image: golangci/golangci-lint:v1.23.1-alpine
+  script:
+  - make test_lint
+  stage: test
+
 pages:
   artifacts:
     paths:

.golangci.yml

@@ -0,0 +1,75 @@
+---
+run:
+  concurrency: 2
+  timeout: 1m
+  issues-exit-code: 1
+  tests: true
+
+output:
+  format: colored-line-number
+  print-issues-lines: true
+  print-linter-name: true
+
+linters-settings:
+  lll:
+    line-length: 140
+
+linters:
+  enable:
+  - bodyclose
+  - deadcode
+  - depguard
+  - dogsled
+  - dupl
+  - errcheck
+  - funlen
+  - gochecknoglobals
+  - gochecknoinits
+  - gocognit
+  - goconst
+  - gocritic
+  - gocyclo
+  - godox
+  - gofmt
+  - goimports
+  - golint
+  - gomnd
+  - goprintffuncname
+  - gosec
+  - gosimple
+  - govet
+  - ineffassign
+  - interfacer
+  - lll
+  - maligned
+  - misspell
+  - nakedret
+  - prealloc
+  - rowserrcheck
+  - scopelint
+  - staticcheck
+  - structcheck
+  - stylecheck
+  - typecheck
+  - unconvert
+  - unparam
+  - unused
+  - varcheck
+  - whitespace
+  - wsl
+  #disable:
+  disabe-all: false
+  fast: false
+
+issues:
+  exclude-rules:
+  - path: version.go
+    linters:
+    - gochecknoglobals
+  - path: _test.go
+    linters:
+    - gomnd
+    - scopelint
+  - path: cases_test.go
+    linters:
+    - gochecknoglobals

Makefile

@@ -12,6 +12,9 @@ test_unit:
 test_cover_report: test_unit
 	@go tool cover -html=cover.out -o code-coverage.html
 
+test_lint:
+	@golangci-lint run --color always
+
 build:
 	@go build -a -v -o $(BIN_FILE)
 

alert.go

@@ -28,35 +28,36 @@ import (
 
 const iconPath string = "assets/icon/tomato.png"
 
-var notifier *notificator.Notificator
-
 // initNotifier initialises the new desktop notifier.
-func initNotifier() {
-	notifier = notificator.New(notificator.Options{
-		DefaultIcon: getIconPath(),
+func initNotifier() *notificator.Notificator {
+	return notificator.New(notificator.Options{
+		DefaultIcon: getNotificationIconPath(),
 		AppName:     "Pominal",
 	})
 }
 
-// getIconPath returns the absolute path of the tomoato icon
+// getNotificationIconPath returns the absolute path of the tomoato icon
 // used for desktop notifications.
 // If there is an error getting the path to the executing program
 // then an empty string is returned.
-func getIconPath() string {
-
+func getNotificationIconPath() string {
 	var result string
+
 	exe, err := os.Executable()
 	if err != nil {
 		fmt.Printf("ERROR: Unable to determine path to this executable. %s", err.Error())
 		return result
 	}
+
 	result = filepath.Dir(exe) + "/" + iconPath
+
 	return result
 }
 
 // alert creates a new desktop notification.
-func alert(l string) {
+func desktopAlert(l string, notifier *notificator.Notificator) {
 	title := "Pominal notification."
 	text := l + " session has started."
-	notifier.Push(title, text, "", notificator.UR_NORMAL)
+
+	_ = notifier.Push(title, text, "", notificator.UR_NORMAL)
 }

cases_test.go

@@ -135,7 +135,7 @@ var sessionsTestCases = []struct {
 	},
 	{
 		name:                "Test session 4",
-		description:         "When the second work session has finished and the long break has started because the maximum number of work sessions is reached.",
+		description:         "When the second work session has finished and the long break has started.",
 		expectedWorkSession: 2,
 		expectedCycle:       1,
 		expectedLabel:       longBreakTimerLabel,

main.go

@@ -20,30 +20,27 @@ package main
 
 import (
 	"flag"
-	"fmt"
+	"log"
 	"os"
 	"time"
 
 	"github.com/rivo/tview"
 )
 
-var (
-	workTime        string
-	shortBreakTime  string
-	longBreakTime   string
-	maxWorkSessions int
-	printVersion    bool
-)
+func main() {
+	var (
+		workTime        string
+		shortBreakTime  string
+		longBreakTime   string
+		maxWorkSessions int
+		printVersion    bool
+	)
 
-func init() {
 	flag.StringVar(&workTime, "work", "25m", "sets the timer for your work session.")
 	flag.StringVar(&shortBreakTime, "short-break", "5m", "sets the timer for your short break.")
 	flag.StringVar(&longBreakTime, "long-break", "20m", "sets the timer for your long break.")
 	flag.IntVar(&maxWorkSessions, "max-work-sessions", 4, "sets the maximum number of work cycles to complete before taking a long break.")
 	flag.BoolVar(&printVersion, "version", false, "print version and exit.")
-}
-
-func main() {
 	flag.Parse()
 
 	if printVersion {
@@ -51,24 +48,19 @@ func main() {
 		os.Exit(0)
 	}
 
-	initNotifier()
-
 	w, err := time.ParseDuration(workTime)
 	if err != nil {
-		fmt.Printf("ERROR: Unable to set the work timer. %s", err.Error())
-		os.Exit(1)
+		log.Fatalf("ERROR: Unable to set the work timer. %s", err.Error())
 	}
 
 	s, err := time.ParseDuration(shortBreakTime)
 	if err != nil {
-		fmt.Printf("ERROR: Unable to set the work timer. %s", err.Error())
-		os.Exit(1)
+		log.Fatalf("ERROR: Unable to set the work timer. %s", err.Error())
 	}
 
 	l, err := time.ParseDuration(longBreakTime)
 	if err != nil {
-		fmt.Printf("ERROR: Unable to set the work timer. %s", err.Error())
-		os.Exit(1)
+		log.Fatalf("ERROR: Unable to set the work timer. %s", err.Error())
 	}
 
 	pominal := NewPominal(
@@ -84,6 +76,7 @@ func main() {
 	flex := newFlex(infoUI, timerUI)
 
 	go pominal.Run(infoUI, timerUI)
+
 	if err := app.SetRoot(flex, false).SetFocus(flex).Run(); err != nil {
 		panic(err)
 	}

pominal.go

@@ -65,11 +65,13 @@ type Pominal struct {
 
 // NewPominal creates a new pominal instance
 func NewPominal(w, s, l float64, m int) Pominal {
+	initWorkSession := 1
+	initCycle := 1
 
 	return Pominal{
-		workSession:     1,
+		workSession:     initWorkSession,
 		maxWorkSessions: setMaxWorkSessions(m),
-		cycle:           1,
+		cycle:           initCycle,
 		work:            setSessionTime(w),
 		shortBreak:      setSessionTime(s),
 		longBreak:       setSessionTime(l),
@@ -79,11 +81,14 @@ func NewPominal(w, s, l float64, m int) Pominal {
 
 // Run Pominal
 func (p *Pominal) Run(infoUI, timerUI *tview.TextView) {
-
 	p.stopChan = make(chan struct{})
 	p.UpdateSession()
 	drawInfo(infoUI, p.cycle, p.workSession, p.maxWorkSessions, p.label)
-	t := time.NewTicker(1 * time.Second)
+
+	// notifier is used for desktop notifications
+	notifier := initNotifier()
+
+	t := time.NewTicker(time.Second)
 
 infinite:
 	for {
@@ -103,9 +108,9 @@ infinite:
 					p.maxWorkSessions,
 					p.label,
 				)
-				alert(p.label)
-				time.Sleep(1 * time.Second)
-				t = time.NewTicker(1 * time.Second)
+				desktopAlert(p.label, notifier)
+				time.Sleep(time.Second)
+				t = time.NewTicker(time.Second)
 			} else {
 				drawTimer(timerUI, p.countdown)
 			}
@@ -158,6 +163,7 @@ func setSessionTime(s float64) float64 {
 	if s < min {
 		return min
 	}
+
 	return s
 }
 
@@ -170,5 +176,6 @@ func setMaxWorkSessions(w int) int {
 	if w < min {
 		return min
 	}
+
 	return w
 }

pominal_test.go

@@ -71,9 +71,6 @@ func TestRun(t *testing.T) {
 	infoUI := tview.NewTextView()
 	timerUI := tview.NewTextView()
 
-	// TODO: Remove when support for disabling desktop notifications is in place.
-	initNotifier()
-
 	go pominal.Run(infoUI, timerUI)
 
 	time.Sleep(1 * time.Second)

ui.go

@@ -56,5 +56,7 @@ func drawInfo(t *tview.TextView, pominalCycle, workSessions, maxWorkSessions int
 
 func drawTimer(t *tview.TextView, countdown float64) {
 	t.Clear()
-	fmt.Fprintf(t, "\nTime remaining:\n%02d:%02d", int(countdown)/60, int(countdown)%60)
+
+	secondsPerMinute := 60
+	fmt.Fprintf(t, "\nTime remaining:\n%02d:%02d", int(countdown)/secondsPerMinute, int(countdown)%secondsPerMinute)
 }