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.

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

Integrate GolangCI-Lint #19

Merged
merged 4 commits into from
Jul 9, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
run:
timeout: 5m
modules-download-mode: readonly

linters-settings:
goconst:
min-len: 2
min-occurrences: 2
gofmt:
simplify: true
goimports:
local-prefixes: github.com/mattermost/mattermost-plugin-aws-SNS
golint:
min-confidence: 0
govet:
check-shadowing: true
enable-all: true
misspell:
locale: US

linters:
disable-all: true
enable:
- bodyclose
- deadcode
- errcheck
- goconst
- gocritic
- gofmt
- goimports
- golint
- gosec
- gosimple
- govet
- ineffassign
- interfacer
- misspell
- nakedret
- staticcheck
- structcheck
- stylecheck
- typecheck
- unconvert
- unused
- varcheck
- whitespace

issues:
exclude-rules:
- path: server/manifest.go
linters:
- deadcode
- varcheck
- path: server/fetcher.go
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just remove fetchOrgMember instead. It's not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I'll fix these, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ended up making the github rate limit check a separate function to be shared. I see some other areas with some duplication as well and will look at trying to clean some of that up.
From an organization standpoint where should I put githubErrorHandle()?

Copy link
Contributor

Choose a reason for hiding this comment

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

A new file github.go would be a good place.

linters:
- unused
- linters:
- goconst
text: "Hit rate limit. Please try again later."
hanzei marked this conversation as resolved.
Show resolved Hide resolved
116 changes: 60 additions & 56 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ GO ?= $(shell command -v go 2> /dev/null)
NPM ?= $(shell command -v npm 2> /dev/null)
CURL ?= $(shell command -v curl 2> /dev/null)
MANIFEST_FILE ?= plugin.json
GOPATH ?= $(shell go env GOPATH)
GO_TEST_FLAGS ?= -race
GO_BUILD_FLAGS ?=
MM_UTILITIES_DIR ?= ../mattermost-utilities

export GO111MODULE=on

# You can include assets this directory into the bundle. This can be e.g. used to include profile pictures.
Expand All @@ -12,6 +17,11 @@ include build/setup.mk

BUNDLE_NAME ?= $(PLUGIN_ID)-$(PLUGIN_VERSION).tar.gz

# Include custom makefile, if present
ifneq ($(wildcard build/custom.mk),)
include build/custom.mk
endif

## Checks the code style, tests, builds and bundles the plugin.
all: check-style test dist

Expand All @@ -20,55 +30,34 @@ all: check-style test dist
apply:
./build/bin/manifest apply

## Runs govet and gofmt against all packages.
## Runs golangci-lint and eslint.
.PHONY: check-style
check-style: webapp/.npminstall gofmt govet
check-style: webapp/.npminstall golangci-lint
@echo Checking for style guide compliance

ifneq ($(HAS_WEBAPP),)
cd webapp && npm run lint
endif

## Runs gofmt against all packages.
.PHONY: gofmt
gofmt:
ifneq ($(HAS_SERVER),)
@echo Running gofmt
@for package in $$(go list ./server/...); do \
echo "Checking "$$package; \
files=$$(go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}} {{end}}' $$package); \
if [ "$$files" ]; then \
gofmt_output=$$(gofmt -d -s $$files 2>&1); \
if [ "$$gofmt_output" ]; then \
echo "$$gofmt_output"; \
echo "Gofmt failure"; \
exit 1; \
fi; \
fi; \
done
@echo Gofmt success
endif

## Runs govet against all packages.
.PHONY: govet
govet:
ifneq ($(HAS_SERVER),)
@echo Running govet
@# Workaroung because you can't install binaries without adding them to go.mod
env GO111MODULE=off $(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
$(GO) vet ./server/...
$(GO) vet -vettool=$(GOPATH)/bin/shadow ./server/...
@echo Govet success
endif
## Run golangci-lint on codebase.
.PHONY: golangci-lint
golangci-lint:
@if ! [ -x "$$(command -v golangci-lint)" ]; then \
echo "golangci-lint is not installed. Please see https://github.com/golangci/golangci-lint#install for installation instructions."; \
exit 1; \
fi; \

@echo Running golangci-lint
golangci-lint run ./...

## Builds the server, if it exists, including support for multiple architectures.
.PHONY: server
server:
ifneq ($(HAS_SERVER),)
mkdir -p server/dist;
cd server && env GOOS=linux GOARCH=amd64 $(GO) build -o dist/plugin-linux-amd64;
cd server && env GOOS=darwin GOARCH=amd64 $(GO) build -o dist/plugin-darwin-amd64;
cd server && env GOOS=windows GOARCH=amd64 $(GO) build -o dist/plugin-windows-amd64.exe;
cd server && env GOOS=linux GOARCH=amd64 $(GO) build $(GO_BUILD_FLAGS) -o dist/plugin-linux-amd64;
cd server && env GOOS=darwin GOARCH=amd64 $(GO) build $(GO_BUILD_FLAGS) -o dist/plugin-darwin-amd64;
cd server && env GOOS=windows GOARCH=amd64 $(GO) build $(GO_BUILD_FLAGS) -o dist/plugin-windows-amd64.exe;
endif

## Ensures NPM dependencies are installed without having to run this all the time.
Expand All @@ -85,6 +74,14 @@ ifneq ($(HAS_WEBAPP),)
cd webapp && $(NPM) run build;
endif

## Builds the webapp in debug mode, if it exists.
.PHONY: webapp-debug
webapp-debug: webapp/.npminstall
ifneq ($(HAS_WEBAPP),)
cd webapp && \
$(NPM) run debug;
endif

## Generates a tar bundle of the plugin for install.
.PHONY: bundle
bundle:
Expand Down Expand Up @@ -114,56 +111,63 @@ endif
dist: apply server webapp bundle

## Installs the plugin to a (development) server.
## It uses the API if appropriate environment variables are defined,
## and otherwise falls back to trying to copy the plugin to a sibling mattermost-server directory.
.PHONY: deploy
deploy: dist
## It uses the API if appropriate environment variables are defined,
## or copying the files directly to a sibling mattermost-server directory.
ifneq ($(and $(MM_SERVICESETTINGS_SITEURL),$(MM_ADMIN_USERNAME),$(MM_ADMIN_PASSWORD),$(CURL)),)
@echo "Installing plugin via API"
$(eval TOKEN := $(shell curl -i --post301 --location $(MM_SERVICESETTINGS_SITEURL) -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/users/login -d '{"login_id": "$(MM_ADMIN_USERNAME)", "password": "$(MM_ADMIN_PASSWORD)"}' | grep Token | cut -f2 -d' ' 2> /dev/null))
@curl -s --post301 --location $(MM_SERVICESETTINGS_SITEURL) -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins -F "plugin=@dist/$(BUNDLE_NAME)" -F "force=true" > /dev/null && \
curl -s --post301 --location $(MM_SERVICESETTINGS_SITEURL) -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins/$(PLUGIN_ID)/enable > /dev/null && \
echo "OK." || echo "Sorry, something went wrong."
else ifneq ($(wildcard ../mattermost-server/.*),)
@echo "Installing plugin via filesystem. Server restart and manual plugin enabling required"
mkdir -p ../mattermost-server/plugins
tar -C ../mattermost-server/plugins -zxvf dist/$(BUNDLE_NAME)
else
@echo "No supported deployment method available. Install plugin manually."
endif
./build/bin/deploy $(PLUGIN_ID) dist/$(BUNDLE_NAME)

.PHONY: debug-deploy
debug-deploy: debug-dist deploy

.PHONY: debug-dist
debug-dist: apply server webapp-debug bundle

## Runs any lints and unit tests defined for the server and webapp, if they exist.
.PHONY: test
test: webapp/.npminstall
ifneq ($(HAS_SERVER),)
$(GO) test -race -v ./server/...
$(GO) test -v $(GO_TEST_FLAGS) ./server/...
endif
ifneq ($(HAS_WEBAPP),)
cd webapp && $(NPM) run fix;
cd webapp && $(NPM) run fix && $(NPM) run test;
endif

## Creates a coverage report for the server code.
.PHONY: coverage
coverage: webapp/.npminstall
ifneq ($(HAS_SERVER),)
$(GO) test -race -coverprofile=server/coverage.txt ./server/...
$(GO) test $(GO_TEST_FLAGS) -coverprofile=server/coverage.txt ./server/...
$(GO) tool cover -html=server/coverage.txt
endif

## Extract strings for translation from the source code.
.PHONY: i18n-extract
i18n-extract:
ifneq ($(HAS_WEBAPP),)
ifeq ($(HAS_MM_UTILITIES),)
@echo "You must clone github.com/mattermost/mattermost-utilities repo in .. to use this command"
else
cd $(MM_UTILITIES_DIR) && npm install && npm run babel && node mmjstool/build/index.js i18n extract-webapp --webapp-dir $(PWD)/webapp
endif
endif

## Clean removes all build artifacts.
.PHONY: clean
clean:
rm -fr dist/
ifneq ($(HAS_SERVER),)
rm -fr server/coverage.txt
rm -fr server/dist
endif
ifneq ($(HAS_WEBAPP),)
rm -fr webapp/.npminstall
rm -fr webapp/junit.xml
rm -fr webapp/dist
rm -fr webapp/node_modules
endif
rm -fr build/bin/

# Help documentatin à la https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html
# Help documentation à la https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html
help:
@cat Makefile | grep -v '\.PHONY' | grep -v '\help:' | grep -B1 -E '^[a-zA-Z0-9_.-]+:.*' | sed -e "s/:.*//" | sed -e "s/^## //" | grep -v '\-\-' | sed '1!G;h;$$!d' | awk 'NR%2{printf "\033[36m%-30s\033[0m",$$0;next;}1' | sort
@cat Makefile | grep -v '\.PHONY' | grep -v '\help:' | grep -B1 -E '^[a-zA-Z0-9_.-]+:.*' | sed -e "s/:.*//" | sed -e "s/^## //" | grep -v '\-\-' | sed '1!G;h;$$!d' | awk 'NR%2{printf "\033[36m%-30s\033[0m",$$0;next;}1' | sort
1 change: 1 addition & 0 deletions build/custom.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Include custome targets and environment variables here
122 changes: 122 additions & 0 deletions build/deploy/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// main handles deployment of the plugin to a development server using either the Client4 API
// or by copying the plugin bundle into a sibling mattermost-server/plugin directory.
package main

import (
"fmt"
"log"
"os"
"path/filepath"

"github.com/mattermost/mattermost-server/v5/model"
"github.com/mholt/archiver/v3"
"github.com/pkg/errors"
)

func main() {
err := deploy()
if err != nil {
fmt.Printf("Failed to deploy: %s\n", err.Error())
fmt.Println()
fmt.Println("Usage:")
fmt.Println(" deploy <plugin id> <bundle path>")
os.Exit(1)
}
}

// deploy handles deployment of the plugin to a development server.
func deploy() error {
if len(os.Args) < 3 {
return errors.New("invalid number of arguments")
}

pluginID := os.Args[1]
bundlePath := os.Args[2]

siteURL := os.Getenv("MM_SERVICESETTINGS_SITEURL")
adminToken := os.Getenv("MM_ADMIN_TOKEN")
adminUsername := os.Getenv("MM_ADMIN_USERNAME")
adminPassword := os.Getenv("MM_ADMIN_PASSWORD")
copyTargetDirectory, _ := filepath.Abs("../mattermost-server")

if siteURL != "" {
client := model.NewAPIv4Client(siteURL)

if adminToken != "" {
log.Printf("Authenticating using token against %s.", siteURL)
client.SetToken(adminToken)

return uploadPlugin(client, pluginID, bundlePath)
}

if adminUsername != "" && adminPassword != "" {
client := model.NewAPIv4Client(siteURL)
log.Printf("Authenticating as %s against %s.", adminUsername, siteURL)
_, resp := client.Login(adminUsername, adminPassword)
if resp.Error != nil {
return errors.Wrapf(resp.Error, "failed to login as %s", adminUsername)
}

return uploadPlugin(client, pluginID, bundlePath)
}
}

_, err := os.Stat(copyTargetDirectory)
if os.IsNotExist(err) {
return errors.New("no supported deployment method available, please install plugin manually")
} else if err != nil {
return errors.Wrapf(err, "failed to stat %s", copyTargetDirectory)
}

log.Printf("Installing plugin to mattermost-server found in %s.", copyTargetDirectory)
log.Print("Server restart required to load updated plugin.")
return copyPlugin(pluginID, copyTargetDirectory, bundlePath)
}

// uploadPlugin attempts to upload and enable a plugin via the Client4 API.
// It will fail if plugin uploads are disabled.
func uploadPlugin(client *model.Client4, pluginID, bundlePath string) error {
pluginBundle, err := os.Open(bundlePath)
if err != nil {
return errors.Wrapf(err, "failed to open %s", bundlePath)
}
defer pluginBundle.Close()

log.Print("Uploading plugin via API.")
_, resp := client.UploadPluginForced(pluginBundle)
if resp.Error != nil {
return errors.Wrap(resp.Error, "failed to upload plugin bundle")
}

log.Print("Enabling plugin.")
_, resp = client.EnablePlugin(pluginID)
if resp.Error != nil {
return errors.Wrap(resp.Error, "Failed to enable plugin")
}

return nil
}

// copyPlugin attempts to install a plugin by copying it to a sibling ../mattermost-server/plugin
// directory. A server restart is required before the plugin will start.
func copyPlugin(pluginID, targetPath, bundlePath string) error {
targetPath = filepath.Join(targetPath, "plugins")

err := os.MkdirAll(targetPath, 0777)
if err != nil {
return errors.Wrapf(err, "failed to create %s", targetPath)
}

existingPluginPath := filepath.Join(targetPath, pluginID)
err = os.RemoveAll(existingPluginPath)
if err != nil {
return errors.Wrapf(err, "failed to remove existing existing plugin directory %s", existingPluginPath)
}

err = archiver.Unarchive(bundlePath, targetPath)
if err != nil {
return errors.Wrapf(err, "failed to unarchive %s into %s", bundlePath, targetPath)
}

return nil
}
Loading