mirror of
https://github.com/opentofu/opentofu.git
synced 2024-12-23 07:33:32 -06:00
Coding standards (#1423)
Signed-off-by: Janos <86970079+janosdebugs@users.noreply.github.com> Signed-off-by: ollevche <ollevche@gmail.com> Co-authored-by: James Humphries <james@james-humphries.co.uk> Co-authored-by: Oleksandr Levchenkov <ollevche@gmail.com>
This commit is contained in:
Janos
GitHub
parent
b7098f50cb
commit
dd5f9afe89
52
.github/pull_request_template.md
vendored
52
.github/pull_request_template.md
vendored
@ -1,32 +1,40 @@
|
||||
<!--
|
||||
|
||||
Describe in detail the changes you are proposing, and the rationale.
|
||||
** Thank you for your contribution! Please read this carefully! **
|
||||
|
||||
See the contributing guide:
|
||||
|
||||
https://github.com/opentofu/opentofu/blob/main/CONTRIBUTING.md
|
||||
Please make sure you go through the checklist below. If your PR does not meet all requirements, please file it
|
||||
as a draft PR. Core team members will only review your PR once it meets all the requirements below (unless your
|
||||
change is something as trivial as a typo fix).
|
||||
|
||||
-->
|
||||
|
||||
<!--
|
||||
|
||||
Link all GitHub issues fixed by this PR, and add references to prior related PRs.
|
||||
Make sure to first open an issue, get community approval and only then create Pull Request to resolve it.
|
||||
All Pull Requests must have an issue attached to them
|
||||
|
||||
-->
|
||||
|
||||
Resolves #
|
||||
<!-- If your PR resolves an issue, please add it here. -->
|
||||
Resolves #
|
||||
|
||||
## Target Release
|
||||
|
||||
<!--
|
||||
|
||||
In normal circumstances we only target changes at the upcoming minor
|
||||
release, or as a patch to the current minor version. If you need to
|
||||
port a security fix to an older release, highlight this here by listing
|
||||
all targeted releases.
|
||||
|
||||
-->
|
||||
|
||||
1.8.0
|
||||
|
||||
## Checklist
|
||||
|
||||
<!-- This checklist is mandatory for all PRs: -->
|
||||
|
||||
- [ ] I have read the [contribution guide](https://github.com/opentofu/opentofu/blob/main/CONTRIBUTING.md).
|
||||
- [ ] I have not used an AI coding assistant to create this PR.
|
||||
- [ ] I have marked any code I have not written myself (including modified code, e.g. copied from other places and then modified) with a comment indicating where it came from.
|
||||
|
||||
### Go checklist
|
||||
|
||||
<-- If your PR contains Go code, please make sure you check off all items on this list: -->
|
||||
|
||||
- [ ] I have run golangci-lint on my change and receive no errors relevant to my code.
|
||||
- [ ] I have run existing tests to ensure my code doesn't break anything.
|
||||
- [ ] I have added tests for all relevant use cases of my code, and those tests are passing.
|
||||
- [ ] I have only exported functions, variables and structs that should be used from other packages.
|
||||
- [ ] I have added meaningful comments to all exported functions, variables, and structs.
|
||||
|
||||
### Website/documentation checklist
|
||||
|
||||
<!-- If you have changed the website, please follow this checklist: -->
|
||||
|
||||
- [ ] I have locally started the website as [described here](https://github.com/opentofu/opentofu/blob/main/website/README.md) and checked my changes.
|
||||
|
||||
7
.github/workflows/checks.yml
vendored
7
.github/workflows/checks.yml
vendored
@ -203,12 +203,17 @@ jobs:
|
||||
|
||||
- name: "Code consistency checks"
|
||||
run: |
|
||||
make fmtcheck importscheck generate staticcheck exhaustive protobuf
|
||||
make generate protobuf
|
||||
if [[ -n "$(git status --porcelain)" ]]; then
|
||||
echo >&2 "ERROR: Generated files are inconsistent. Run 'make generate' and 'make protobuf' locally and then commit the updated files."
|
||||
git >&2 status --porcelain
|
||||
exit 1
|
||||
fi
|
||||
- name: "Code linting"
|
||||
uses: golangci/golangci-lint-action@v6
|
||||
with:
|
||||
version: v1.58
|
||||
only-new-issues: true
|
||||
|
||||
license-checks:
|
||||
name: "License Checks"
|
||||
|
||||
89
.golangci.yml
Normal file
89
.golangci.yml
Normal file
@ -0,0 +1,89 @@
|
||||
run:
|
||||
timeout: 30m
|
||||
|
||||
linters-settings:
|
||||
errcheck:
|
||||
check-type-assertions: true
|
||||
|
||||
exhaustive:
|
||||
check:
|
||||
- switch
|
||||
- map
|
||||
|
||||
funlen:
|
||||
lines: 100
|
||||
statements: 50
|
||||
ignore-comments: true
|
||||
|
||||
govet:
|
||||
enable-all: true
|
||||
disable:
|
||||
- fieldalignment # rule is too strict
|
||||
|
||||
nolintlint:
|
||||
require-explanation: true
|
||||
require-specific: true
|
||||
|
||||
linters:
|
||||
disable-all: true
|
||||
enable:
|
||||
- asasalint
|
||||
- asciicheck
|
||||
- bidichk
|
||||
- bodyclose
|
||||
- cyclop
|
||||
- dupl
|
||||
- durationcheck
|
||||
- errcheck
|
||||
- errname
|
||||
- errorlint
|
||||
- exhaustive
|
||||
- exportloopref
|
||||
- forbidigo
|
||||
- funlen
|
||||
- gocheckcompilerdirectives
|
||||
- gochecknoglobals
|
||||
- gochecknoinits
|
||||
- gocognit
|
||||
- goconst
|
||||
- gocritic
|
||||
- gocyclo
|
||||
- goimports
|
||||
- gomoddirectives
|
||||
- gomodguard
|
||||
- goprintffuncname
|
||||
- gosec
|
||||
- gosimple
|
||||
- govet
|
||||
- ineffassign
|
||||
- loggercheck
|
||||
- makezero
|
||||
- mirror
|
||||
- mnd
|
||||
- musttag
|
||||
- nakedret
|
||||
- nestif
|
||||
- nilerr
|
||||
- nilnil
|
||||
- noctx
|
||||
- nolintlint
|
||||
- nonamedreturns
|
||||
- nosprintfhostport
|
||||
- predeclared
|
||||
- promlinter
|
||||
- reassign
|
||||
- revive
|
||||
- rowserrcheck
|
||||
- sqlclosecheck
|
||||
- staticcheck
|
||||
- stylecheck
|
||||
- tenv
|
||||
- testableexamples
|
||||
- tparallel
|
||||
- typecheck
|
||||
- unconvert
|
||||
- unparam
|
||||
- unused
|
||||
- usestdlibvars
|
||||
- wastedassign
|
||||
- whitespace
|
||||
@ -37,11 +37,41 @@ We specifically do not merge PRs **without prior issues** that:
|
||||
Eager to get started on coding? Here's the short version:
|
||||
|
||||
1. Set up a Go development environment with Git.
|
||||
2. Pay attention to copyright: [please read the DCO](https://developercertificate.org/), write the code yourself, avoid copy/paste. Disable or limit your AI coding assistant.
|
||||
2. Pay attention to copyright: [please read the DCO](https://developercertificate.org/), write the code yourself, avoid copy/paste. Disable your AI coding assistant.
|
||||
3. Run the tests with `go test` in the package you are working on.
|
||||
4. Build OpenTofu by running `go build ./cmd/tofu`.
|
||||
5. Update [the changelog](CHANGELOG.md).
|
||||
6. When you commit, use `git commit -s` to sign off your commits.
|
||||
7. Complete the checklist below before you submit your PR (or submit a draft PR).
|
||||
8. Your PR will be reviewed by the core team once it is marked as ready to review.
|
||||
|
||||
---
|
||||
|
||||
## PR checklist
|
||||
|
||||
<!-- Make sure to keep this in sync with the PR template. -->
|
||||
|
||||
Please make sure you complete the following checklist before you mark your PR ready for review. If you cannot complete the checklist but want to submit a PR, please submit it as a draft PR. Please note, the core team will only review your PR if you have completed the checklist and marked your PR as ready to review.
|
||||
|
||||
- [ ] I have read the contribution guidelines.
|
||||
- [ ] I have not used an AI coding assistant to create this PR.
|
||||
- [ ] I have marked any code I have not written myself (including modified code, e.g. copied from other places and then modified) with a comment indicating where it came from.
|
||||
|
||||
### Go checklist
|
||||
|
||||
If your PR contains Go code, please make sure you check off all items on this list:
|
||||
|
||||
- [ ] I have run golangci-lint on my change and receive no errors relevant to my code.
|
||||
- [ ] I have run existing tests to ensure my code doesn't break anything.
|
||||
- [ ] I have added tests for all relevant use cases of my code, and those tests are passing.
|
||||
- [ ] I have only exported functions, variables and structs that should be used from other packages.
|
||||
- [ ] I have added meaningful comments to all exported functions, variables, and structs.
|
||||
|
||||
### Website/documentation checklist
|
||||
|
||||
If you have changed the website, please follow this checklist:
|
||||
|
||||
- [ ] I have locally started the website as [described here](https://github.com/opentofu/opentofu/blob/main/website/README.md) and checked my changes.
|
||||
|
||||
---
|
||||
|
||||
@ -224,7 +254,7 @@ We take copyright and intellectual property very seriously. A few quick rules sh
|
||||
|
||||
1. When you submit a PR, you are responsible for the code in that pull request. You signal your acceptance of the [DCO](https://developercertificate.org/) with your sign-off.
|
||||
2. If you include code in your PR that you didn't write yourself, make sure you have permission from the author. If you have permission, always add the `Co-authored-by` sign-off to your commits to indicate the author of the code you are adding.
|
||||
3. Be careful about AI coding assistants! Coding assistants based on large language models (LLMs), such as ChatGPT or GitHub Copilot, are awesome tools to help. However, in the specific case of OpenTofu the training data may include the BSL-licensed Terraform. Since the OpenTofu/Terraform codebase is very specific and LLMs don't have any other training sources, they may emit copyrighted code. Please avoid using LLM-based coding assistants as much as possible.
|
||||
3. Be careful about AI coding assistants! Coding assistants based on large language models (LLMs), such as ChatGPT or GitHub Copilot, are awesome tools to help. However, in the specific case of OpenTofu the training data may include the BSL-licensed Terraform. Since the OpenTofu/Terraform codebase is very specific and LLMs don't have any other training sources, they may emit copyrighted code. Please avoid using LLM-based coding assistants.
|
||||
4. When you copy/paste code from within the OpenTofu code, always make it explicit where you copied from. This helps us resolve issues later on.
|
||||
5. Before you copy code from external sources, make sure that the license allows this. Also make sure that any licensing requirements, such as attribution, are met. When in doubt, ask first!
|
||||
6. Specifically, do not copy from the Terraform repository, or any PRs others have filed against that repository. This code is licensed under the BSL, a license which is not compatible with OpenTofu. (You may submit the same PR to both Terraform and OpenTofu as long as you are the author of both.)
|
||||
|
||||
20
Makefile
20
Makefile
@ -21,21 +21,11 @@ generate:
|
||||
protobuf:
|
||||
go run ./tools/protobuf-compile .
|
||||
|
||||
.PHONY: fmtcheck
|
||||
fmtcheck:
|
||||
"$(CURDIR)/scripts/gofmtcheck.sh"
|
||||
|
||||
.PHONY: importscheck
|
||||
importscheck:
|
||||
"$(CURDIR)/scripts/goimportscheck.sh"
|
||||
|
||||
.PHONY: staticcheck
|
||||
staticcheck:
|
||||
"$(CURDIR)/scripts/staticcheck.sh"
|
||||
|
||||
.PHONY: exhaustive
|
||||
exhaustive:
|
||||
"$(CURDIR)/scripts/exhaustive.sh"
|
||||
# Golangci-lint
|
||||
.PHONY: golangci-lint
|
||||
golangci-lint:
|
||||
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.58.1
|
||||
golangci-lint run --timeout 60m ./...
|
||||
|
||||
# Run license check
|
||||
.PHONY: license-check
|
||||
|
||||
@ -1,12 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
# Copyright (c) The OpenTofu Authors
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
# Copyright (c) 2023 HashiCorp, Inc.
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
|
||||
|
||||
echo "==> Checking for switch statement exhaustiveness..."
|
||||
|
||||
# For now we're only checking a handful of packages, rather than defaulting to
|
||||
# everything with a skip list.
|
||||
go run github.com/nishanths/exhaustive/cmd/exhaustive ./internal/command/views/json
|
||||
@ -1,18 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
# Copyright (c) The OpenTofu Authors
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
# Copyright (c) 2023 HashiCorp, Inc.
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
|
||||
|
||||
# Check go fmt
|
||||
echo "==> Checking that code complies with go fmt requirements..."
|
||||
gofmt_files=$(go fmt ./...)
|
||||
if [[ -n ${gofmt_files} ]]; then
|
||||
echo 'gofmt needs running on the following files:'
|
||||
echo "${gofmt_files}"
|
||||
echo "You can use the command: \`go fmt\` to reformat code."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
exit 0
|
||||
@ -1,83 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
# Copyright (c) The OpenTofu Authors
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
# Copyright (c) 2023 HashiCorp, Inc.
|
||||
# SPDX-License-Identifier: MPL-2.0
|
||||
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
# Check goimports
|
||||
echo "==> Checking the code complies with goimports requirements..."
|
||||
|
||||
# We only require goimports to have been run on files that were changed
|
||||
# relative to the main branch, so that we can gradually create more consistency
|
||||
# rather than bulk-changing everything at once.
|
||||
|
||||
declare -a target_files
|
||||
# "readarray" will return an "unbound variable" error if there isn't already
|
||||
# at least one element in the target array. "readarray" will overwrite this
|
||||
# item, though.
|
||||
target_files[0]=""
|
||||
|
||||
base_branch="origin/main"
|
||||
|
||||
# HACK: If we seem to be running inside a GitHub Actions pull request check
|
||||
# then we'll use the PR's target branch from this variable instead.
|
||||
if [[ -n "${GITHUB_BASE_REF:-}" ]]; then
|
||||
base_branch="origin/$GITHUB_BASE_REF"
|
||||
fi
|
||||
|
||||
# FIXME: "readarray' is a Bash 4 feature, which means that currently this script
|
||||
# can't work on macOS which (at the time of writing this) ships with only Bash 3.
|
||||
# We can probably replace this with something more clunky using an overridden
|
||||
# "IFS" environment variable, but the primary place we want to run this right
|
||||
# now is in our "quick checks" workflow and that _does_ have a reasonably
|
||||
# modern version of Bash.
|
||||
readarray -t target_files < <(git diff --name-only ${base_branch} --diff-filter=MA | grep "\.go" | grep -v ".pb.go" | grep -v ".go-version" | grep -v ".goreleaser")
|
||||
|
||||
# NOTE: The above intentionally excludes .pb.go files because those are
|
||||
# generated by a tool (protoc-gen-go) which itself doesn't produce
|
||||
# style-compliant imports.
|
||||
|
||||
if [[ "${#target_files[@]}" -eq 0 ]]; then
|
||||
echo "No files have changed relative to branch ${base_branch}, so there's nothing to check!"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
declare -a incorrect_files
|
||||
# Array must have at least one item before we can append to it. Code below must
|
||||
# work around this extra empty-string element at the beginning of the array.
|
||||
incorrect_files[0]=""
|
||||
|
||||
for filename in "${target_files[@]}"; do
|
||||
if [[ -z "$filename" ]]; then
|
||||
continue
|
||||
fi
|
||||
|
||||
output=$(go run golang.org/x/tools/cmd/goimports -l "${filename}")
|
||||
if [[ $? -ne 0 ]]; then
|
||||
echo >&2 goimports failed for "$filename"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [[ -n "$output" ]]; then
|
||||
incorrect_files+=("$filename")
|
||||
fi
|
||||
done
|
||||
|
||||
if [[ "${#incorrect_files[@]}" -gt 1 ]]; then
|
||||
echo >&2 'The following files have import statements that disagree with "goimports"':
|
||||
for filename in "${incorrect_files[@]}"; do
|
||||
if [[ -z "$filename" ]]; then
|
||||
continue
|
||||
fi
|
||||
|
||||
echo >&2 ' - ' "${filename}"
|
||||
done
|
||||
echo >&2 'Use `go run golang.org/x/tools/cmd/goimports -w -l` on each of these files to update these files.'
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo 'All of the changed files look good!'
|
||||
exit 0
|
||||
Loading…
Reference in New Issue
Block a user