jakekgrog / GhostDB

GhostDB is a distributed, in-memory, general purpose key-value data store that delivers microsecond performance at any scale.
http://www.ghostdbcache.com
BSD 3-Clause "New" or "Revised" License
750 stars 44 forks source link

Add linting phase to workflow #19

Closed jakekgrog closed 4 years ago

jakekgrog commented 4 years ago

Adding a linting phase to our workflow will ensure all future changes conform to the same style standard.

tegk commented 4 years ago

I would suggest to use either https://github.com/golangci/golangci-lint-action or https://github.com/github/super-linter with following linters enabled:

govet
errcheck 
staticcheck 
unused 
gosimple 
structcheck
varcheck
ineffassign
deadcode
typecheck
bodyclose
gofmt 
goimports
misspell 
unparam 
whitespace
unconvert

These might be useful too:

goconst 
prealloc
nakedret
dogsled

The full list can be viewed here: https://golangci-lint.run/usage/linters/

jakekgrog commented 4 years ago

Add linter - https://github.com/jakekgrog/GhostDB/pull/20

We can refine the lint tests in another PR, @tegk if you want to take this maybe? You seem to be very familiar with this linter?

tegk commented 4 years ago

With the suggested linter enabled this could look like this: .github/workflows/linter.yml

###########################
###########################
## Linter GitHub Actions ##
###########################
###########################
name: Lint Code Base

#
# Documentation:
# https://help.github.com/en/articles/workflow-syntax-for-github-actions
#

#############################
# Start the job on all push #
#############################
on:
  push:
    branches-ignore: [master]
    # Remove the line above to run when pushing to master
  pull_request:
    branches: [master]

###############
# Set the Job #
###############
jobs:
  build:
    # Name the Job
    name: Lint Code Base
    # Set the agent to run on
    runs-on: ubuntu-latest

    ##################
    # Load all steps #
    ##################
    steps:
      ##########################
      # Checkout the code base #
      ##########################
      - name: Checkout Code
        uses: actions/checkout@v2

      ################################
      # Run Linter against code base #
      ################################
      - name: Lint Code Base
        uses: docker://github/super-linter:v3
        env:
          VALIDATE_ALL_CODEBASE: false
          DEFAULT_BRANCH: master
          VALIDATE_GO: true

.github/linters/.golangci.yml

#########################
#########################
## Golang Linter rules ##
#########################
#########################

# configure golangci-lint
issues:
  exclude-rules:
    - path: _test\.go
      linters:
      - dupl
      - goconst
linters:
  enable:
    - golint
    - govet
    - errcheck 
    - staticcheck 
    - unused 
    - gosimple 
    - structcheck
    - varcheck
    - ineffassign
    - deadcode
    - typecheck
    - bodyclose
    - gofmt 
    - goimports
    - misspell 
    - unparam 
    - whitespace
    - unconvert
    - goconst 
    - prealloc
    - nakedret
    - dogsled

linters-settings:
  errcheck:
    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
    # default is false: such cases aren't reported by default.
    check-blank: true

But feel free to have a look at https://golangci-lint.run/usage/linters/ to see which linter you would want to enforce.

jakekgrog commented 4 years ago

@tegk golangci-lint is reporting different issues locally vs in PRs, not picking up that certain linters should not run in test files and is ignoring other pieces of linter config. I am considering removing this linter until we can find one that exhibits the same behaviour locally and remotely.

jakekgrog commented 4 years ago

If I'm missing something here, please point it out and open a new issue/PR and indicate how to run locally as I could not get local version to output the same results as results from github actions.