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

Enable more linters #252

Merged
merged 19 commits into from
May 22, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
50 changes: 40 additions & 10 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,35 +1,65 @@
service:
golangci-lint-version: 1.23.0

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-github
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
# TODO: enable this later
# - errcheck
- whitespace

issues:
exclude-rules:
- linters:
# ignore unused warning for manifest
- varcheck
- deadcode
text: "manifest"
- path: server/manifest.go
linters:
- deadcode
- unused
- varcheck
- path: server/configuration.go
linters:
- unused
- path: _test\.go
linters:
- bodyclose
- goconst
- scopelint # https://github.com/kyoh86/scopelint/issues/4
- path: server/webhook.go
linters:
- goconst
Comment on lines +63 to +65
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are so many action types like "opened" in server/webhook.go that refactoring them into const's did make the code more readable.

72 changes: 44 additions & 28 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ 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
Expand All @@ -14,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 @@ -22,7 +30,7 @@ all: check-style test dist
apply:
./build/bin/manifest apply

## Runs golangci-lint against all packages.
## Runs golangci-lint and eslint.
.PHONY: check-style
check-style: webapp/.npminstall golangci-lint
@echo Checking for style guide compliance
Expand All @@ -31,8 +39,9 @@ ifneq ($(HAS_WEBAPP),)
cd webapp && npm run lint
endif

golangci-lint: ## Run golangci-lint on codebase
# https://stackoverflow.com/a/677212/1027058 (check if a command exists or not)
## 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; \
Expand All @@ -46,9 +55,9 @@ golangci-lint: ## Run golangci-lint on codebase
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 @@ -65,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 @@ -94,64 +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 -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 -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins -F "plugin=@dist/$(BUNDLE_NAME)" -F "force=true" > /dev/null && \
curl -s -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:
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),)
@[[ -d $(MM_UTILITIES_DIR) ]] || echo "You must clone github.com/mattermost/mattermost-utilities repo in .. to use this command"
@[[ -d $(MM_UTILITIES_DIR) ]] && cd $(MM_UTILITIES_DIR) && npm install && npm run babel && node mmjstool/build/index.js i18n extract-webapp --webapp-dir ../mattermost-plugin-demo/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
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
agnivade marked this conversation as resolved.
Show resolved Hide resolved
// 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