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

Add set of requirements for PR and add go linter tools for checking security, format, simplify... #443

Merged
merged 5 commits into from
Apr 23, 2022
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
5 changes: 5 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ By submitting this pull request, I confirm that you can use, modify, copy, and r
# Tests
_Describe what tests you have done._

#Requirements
_Before commit the code, please do the following steps._
1. Run `make fmt` and `make fmt-sh`
2. Run `make linter`




61 changes: 61 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
run:
# The default concurrency value is the number of available CPU.
concurrency: 4

# Timeout for analysis, e.g. 30s, 5m, default is 1m
timeout: 20m

# Exit code when at least one issue was found, default is 1
issues-exit-code: 1

# Include test files or not, default is true
tests: true

# If invoked with -mod=readonly, the go command is disallowed from the implicit
# automatic updating of go.mod described above. Instead, it fails when any changes
# to go.mod are needed. This setting is most useful to check that go.mod does
# not need updates, such as in a continuous integration and testing system.
modules-download-mode: readonly

output:
# colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
format: colored-line-number

# print lines of code with issue, default is true
print-issued-lines: true

# print linter name in the end of issue text, default is true
print-linter-name: true

# All available settings of specific linters
linters-settings:
revive:
# minimal confidence for issues, default is 0.8
min-confidence: 0.7
gofmt:
# Simplify code: gofmt with `-s` option, true by default
simplify: true
goimports:
# Put imports beginning with prefix after 3rd-party packages.
# It's a comma-separated list of prefixes.
local-prefixes: github.com/aws/amazon-cloudwatch-agent
misspell:
# Correct spellings using locale preferences for US or UK.
# Default is to use a neutral variety of English.
# Setting locale to US will correct the British spelling of 'colour' to 'color'.
locale: US

linters:
disable:
- errcheck
- gofmt
- goimports
enable:
- gosimple
- deadcode
- gosec
- unused
- structcheck
- misspell
- revive
- golint
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ GOIMPORTS_OPT?= -w -local $(CW_AGENT_IMPORT_PATH)

GOIMPORTS = $(TOOLS_BIN_DIR)/goimports
SHFMT = $(TOOLS_BIN_DIR)/shfmt

LINTER = $(TOOLS_BIN_DIR)/golangci-lint
release: clean test build package-rpm package-deb package-win package-darwin

nightly-release: release
Expand Down Expand Up @@ -128,9 +128,12 @@ build-for-docker-arm64:
$(LINUX_ARM64_BUILD)/start-amazon-cloudwatch-agent github.com/aws/amazon-cloudwatch-agent/cmd/start-amazon-cloudwatch-agent
$(LINUX_ARM64_BUILD)/config-translator github.com/aws/amazon-cloudwatch-agent/cmd/config-translator

#Install from source for golangci-lint is not recommended based on https://golangci-lint.run/usage/install/#install-from-source so using binary
#installation
install-tools:
GOBIN=$(TOOLS_BIN_DIR) go install golang.org/x/tools/cmd/goimports
GOBIN=$(TOOLS_BIN_DIR) go install mvdan.cc/sh/v3/cmd/shfmt@latest
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(TOOLS_BIN_DIR) v1.45.2

fmt: install-tools
go fmt ./...
Expand All @@ -139,6 +142,9 @@ fmt: install-tools
fmt-sh: install-tools
${SHFMT} -w -d -i 5 .

lint: install-tools
${LINTER} run ./...
Copy link
Contributor

@aateeqi aateeqi Apr 20, 2022

Choose a reason for hiding this comment

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

why are we not just using golint as described here: https://sparkbox.com/foundry/go_vet_gofmt_golint_to_code_check_in_Go

Copy link
Contributor Author

@khanhntd khanhntd Apr 20, 2022

Choose a reason for hiding this comment

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

This includes go-lint but it disables by default and I have enabled it again.


test:
CGO_ENABLED=0 go test -coverprofile coverage.txt -failfast ./awscsm/... ./cfg/... ./cmd/... ./handlers/... ./internal/... ./logger/... ./logs/... ./metric/... ./plugins/... ./profiler/... ./tool/... ./translator/...

Expand Down