diff --git a/.codecov.yml b/.codecov.yml index 0f63adb3a6c..2ef223a6e96 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -3,3 +3,5 @@ ignore: - "github/github-accessors.go" # ignore experimental scrape package - "scrape" + # ignore update-urls + - "update-urls" diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index 4bc27ab6f2d..98a1f5de8be 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -3,30 +3,14 @@ name: linter permissions: contents: read - pull-requests: read jobs: lint: - strategy: - matrix: - platform: [ubuntu-latest] - - # golangci-lint will only process a single module, so we need to call it - # separately for each module in the repo. We dont lint example/newreposecretwithlibsodium - # since that needs libsodium to run. - working-directory: - - "" - - example - - scrape - - update-urls - runs-on: ${{ matrix.platform }} - + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - - name: golangci-lint ${{ matrix.working-directory }} - uses: golangci/golangci-lint-action@v3 + - uses: actions/setup-go@v4 with: - version: latest - working-directory: ${{ matrix.working-directory}} - args: --verbose --timeout=10m + go-version: 1.x + cache-dependency-path: "**/go.sum" + - run: script/lint.sh diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 87d97f99c09..b3abd8b6f92 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,6 +19,9 @@ permissions: jobs: test: + defaults: + run: + shell: bash strategy: matrix: go-version: [1.x, 1.20.x] @@ -41,12 +44,11 @@ jobs: go-version: ${{ matrix.go-version }} - uses: actions/checkout@v4 - # Get values for cache paths to be used in later steps + # Get values for cache paths to be used in later steps - id: cache-paths run: | echo "go-cache=$(go env GOCACHE)" >> $GITHUB_OUTPUT echo "go-mod-cache=$(go env GOMODCACHE)" >> $GITHUB_OUTPUT - shell: bash - name: Cache go modules uses: actions/cache@v3 @@ -57,29 +59,18 @@ jobs: key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} restore-keys: ${{ runner.os }}-go- - - name: Ensure go generate produces a zero diff - shell: bash - run: go generate -x ./... && git diff --exit-code; code=$?; git checkout -- .; (exit $code) - - name: Run go test - run: go test -v -race -coverprofile coverage.txt -covermode atomic ./... + run: | + if [ -n "${{ matrix.update-coverage }}" ]; then + script/test.sh -race -covermode atomic -coverprofile coverage.txt ./... + exit + fi + script/test.sh -race -covermode atomic ./... - name: Ensure integration tests build # don't actually run tests since they hit live GitHub API run: go test -v -tags=integration -run=^$ ./test/integration - - name: Run scrape tests - run: | - cd scrape - go test ./... - - name: Upload coverage to Codecov if: ${{ matrix.update-coverage }} uses: codecov/codecov-action@eaaf4bedf32dbdc6b720b63067d99c4d77d6047d #v3.1.4 - - - name: Ensure go generate produces a zero diff for update-urls - shell: bash - run: cd update-urls && go generate -x ./... && git diff --exit-code; code=$?; git checkout -- .; (exit $code) - - - name: Run go test for update-urls - run: cd update-urls && go test -v -race ./... diff --git a/.gitignore b/.gitignore index 704f4ef5b54..0c803b53aa6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ *.sh +!/script/*.sh *.test coverage.out +/bin # intellij files .idea/ vendor/ diff --git a/.golangci.yml b/.golangci.yml index 3e286f045f4..d1ec5b11c92 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,7 @@ run: build-tags: - integration + timeout: 10m linters: enable: - dogsled diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fb3660461fc..451924ce4c3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,68 +31,62 @@ are more sensitive, emailed to . ## Submitting a patch ## - 1. It's generally best to start by opening a new issue describing the bug or - feature you're intending to fix. Even if you think it's relatively minor, - it's helpful to know what people are working on. Mention in the initial - issue that you are planning to work on that bug or feature so that it can - be assigned to you. - - 1. Follow the normal process of [forking][] the project, and setup a new - branch to work in. It's important that each group of changes be done in - separate branches in order to ensure that a pull request only includes the - commits related to that bug or feature. - - 1. Go makes it very simple to ensure properly formatted code, so always run - `go fmt` on your code before committing it. You should also run - [go vet][] over your code. this will help you find common style issues - within your code and will keep styling consistent within the project. - - 1. Any significant changes should almost always be accompanied by tests. The - project already has good test coverage, so look at some of the existing - tests if you're unsure how to go about it. [gocov][] and [gocov-html][] - are invaluable tools for seeing which parts of your code aren't being - exercised by your tests. - - 1. Please run: - * `go generate github.com/google/go-github/...` - * `go test github.com/google/go-github/...` - * `go vet github.com/google/go-github/...` - - The `go generate ./...` command will update or generate certain files, and the - resulting changes should be included in your pull request. - - The `go test ./...` command will run tests inside your code. This will help you - spot places where code might be faulty before committing. - - And finally, the `go vet ./...` command will check linting and styling over your - code, keeping the project consistent formatting-wise. - - In any case, it is always a good idea to read [official Go documentation][] when working - on this project, as the definition of tools and commands of the Go programming - language is described in further detail there. - - 1. Do your best to have [well-formed commit messages][] for each change. - This provides consistency throughout the project, and ensures that commit - messages are able to be formatted properly by various git tools. - - 1. Finally, push the commits to your fork and submit a [pull request][]. - Before pushing commits, it is highly advised to check for generated files - that were either created or modified for the sake of your commit. Running - `go generate -x ./...` should return a log of modified generated files that should - be included alongside the manually written code in the commit. - **NOTE:** Please do not use force-push on PRs in this repo, as it makes - it more difficult for reviewers to see what has changed since the last - code review. - -[official Go documentation]: https://pkg.go.dev/std +1. It's generally best to start by opening a new issue describing the bug or + feature you're intending to fix. Even if you think it's relatively minor, + it's helpful to know what people are working on. Mention in the initial issue + that you are planning to work on that bug or feature so that it can be + assigned to you. + +2. Follow the normal process of [forking][] the project, and set up a new branch + to work in. It's important that each group of changes be done in separate + branches in order to ensure that a pull request only includes the commits + related to that bug or feature. + +3. Any significant changes should almost always be accompanied by tests. The + project already has good test coverage, so look at some of the existing tests + if you're unsure how to go about it. Coverage is [monitored by codecov.io][], + which flags pull requests that decrease test coverage. This doesn't + necessarily mean that PRs with decreased coverage won't be merged. Sometimes + a decrease in coverage makes sense, but if your PR is flagged, you should + either add tests to cover those lines or add a PR comment explaining the + untested lines. + +4. Run `script/fmt.sh`, `script/test.sh` and `script/lint.sh` to format your code and + check that it passes all tests and linters. `script/lint.sh` may also tell you + that generated files need to be updated. If so, run `script/generate.sh` to + update them. + +5. Do your best to have [well-formed commit messages][] for each change. This + provides consistency throughout the project, and ensures that commit messages + are able to be formatted properly by various git tools. + +6. Finally, push the commits to your fork and submit a [pull request][]. + **NOTE:** Please do not use force-push on PRs in this repo, as it makes it + more difficult for reviewers to see what has changed since the last code + review. We always perform "squash and merge" actions on PRs in this repo, so it doesn't + matter how many commits your PR has, as they will end up being a single commit after merging. + This is done to make a much cleaner `git log` history and helps to find regressions in the code + using existing tools such as `git bisect`. + [forking]: https://help.github.com/articles/fork-a-repo -[go vet]: https://pkg.go.dev/cmd/vet -[gocov]: https://github.com/axw/gocov -[gocov-html]: https://github.com/matm/gocov-html [well-formed commit messages]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html -[squash]: http://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits [pull request]: https://help.github.com/articles/creating-a-pull-request +[monitored by codecov.io]: https://codecov.io/gh/google/go-github + +## Scripts ## + +The `script` directory has shell scripts that help with common development +tasks. + +**script/fmt.sh** formats all go code in the repository. + +**script/generate.sh** runs code generators and `go mod tidy` on all modules. With +`--check` it checks that the generated files are current. + +**script/lint.sh** runs linters on the project and checks generated files are +current. +**script/test.sh** runs tests on all modules. ## Other notes on code organization ## @@ -144,5 +138,5 @@ this][modified-comment]. [rebase-comment]: https://github.com/google/go-github/pull/277#issuecomment-183035491 [modified-comment]: https://github.com/google/go-github/pull/280#issuecomment-184859046 -**When creating a release, don't forget to update the `Version` constant in `github.go`.** This is used to +**When creating a release, don't forget to update the `Version` constant in `github.go`.** This is used to send the version in the `User-Agent` header to identify clients to the GitHub API. diff --git a/example/basicauth/main.go b/example/basicauth/main.go index e677aa94a42..95ec4360c21 100644 --- a/example/basicauth/main.go +++ b/example/basicauth/main.go @@ -20,7 +20,6 @@ import ( "fmt" "os" "strings" - "syscall" "github.com/google/go-github/v55/github" "golang.org/x/term" @@ -32,7 +31,7 @@ func main() { username, _ := r.ReadString('\n') fmt.Print("GitHub Password: ") - bytePassword, _ := term.ReadPassword(syscall.Stdin) + bytePassword, _ := term.ReadPassword(int(os.Stdin.Fd())) password := string(bytePassword) tp := github.BasicAuthTransport{ diff --git a/example/tagprotection/main.go b/example/tagprotection/main.go index 3b9b723dd0f..4127d2b44de 100644 --- a/example/tagprotection/main.go +++ b/example/tagprotection/main.go @@ -17,7 +17,6 @@ import ( "log" "os" "strings" - "syscall" "github.com/google/go-github/v55/github" "golang.org/x/term" @@ -39,7 +38,7 @@ func main() { pattern = strings.TrimSpace(pattern) fmt.Print("GitHub Token: ") - byteToken, _ := term.ReadPassword(syscall.Stdin) + byteToken, _ := term.ReadPassword(int(os.Stdin.Fd())) println() token := string(byteToken) diff --git a/example/tokenauth/main.go b/example/tokenauth/main.go index 91ad5e5067a..ccd36543584 100644 --- a/example/tokenauth/main.go +++ b/example/tokenauth/main.go @@ -13,7 +13,7 @@ import ( "context" "fmt" "log" - "syscall" + "os" "github.com/google/go-github/v55/github" "golang.org/x/term" @@ -21,7 +21,7 @@ import ( func main() { fmt.Print("GitHub Token: ") - byteToken, _ := term.ReadPassword(syscall.Stdin) + byteToken, _ := term.ReadPassword(int(os.Stdin.Fd())) println() token := string(byteToken) diff --git a/script/fmt.sh b/script/fmt.sh new file mode 100755 index 00000000000..20ff6e69254 --- /dev/null +++ b/script/fmt.sh @@ -0,0 +1,15 @@ +#!/bin/sh +#/ script/fmt.sh runs go fmt on all go files in the project. + +set -e + +CDPATH="" cd -- "$(dirname -- "$0")/.." + +MOD_DIRS="$(git ls-files '*go.mod' | xargs dirname | sort)" + +for dir in $MOD_DIRS; do + ( + cd "$dir" + go fmt ./... + ) +done diff --git a/script/generate.sh b/script/generate.sh new file mode 100755 index 00000000000..17707b2385c --- /dev/null +++ b/script/generate.sh @@ -0,0 +1,48 @@ +#!/bin/sh +#/ script/generate.sh runs go generate on all modules in this repo. +#/ `script/generate.sh --check` checks that the generated files are up to date. + +set -e + +CDPATH="" cd -- "$(dirname -- "$0")/.." + +if [ "$1" = "--check" ]; then + GENTEMP="$(mktemp -d)" + git worktree add -q --detach "$GENTEMP" + trap 'git worktree remove -f "$GENTEMP"; rm -rf "$GENTEMP"' EXIT + for f in $(git ls-files -com --exclude-standard); do + target="$GENTEMP/$f" + mkdir -p "$(dirname -- "$target")" + cp "$f" "$target" + done + if [ -f "$(pwd)"/bin ]; then + ln -s "$(pwd)"/bin "$GENTEMP"/bin + fi + ( + cd "$GENTEMP" + git add . + git -c user.name='bot' -c user.email='bot@localhost' commit -m "generate" -q --allow-empty + script/generate.sh + [ -z "$(git status --porcelain)" ] || { + msg="Generated files are out of date. Please run script/generate.sh and commit the results" + if [ -n "$GITHUB_ACTIONS" ]; then + echo "::error ::$msg" + else + echo "$msg" 1>&2 + fi + git diff + exit 1 + } + ) + exit 0 +fi + +MOD_DIRS="$(git ls-files '*go.mod' | xargs dirname | sort)" + +for dir in $MOD_DIRS; do + ( + cd "$dir" + go generate ./... + go mod tidy -compat '1.17' + ) +done diff --git a/script/lint.sh b/script/lint.sh new file mode 100755 index 00000000000..af5ae991006 --- /dev/null +++ b/script/lint.sh @@ -0,0 +1,38 @@ +#!/bin/sh +#/ script/lint.sh runs linters and validates generated files. + +set -e + +GOLANGCI_LINT_VERSION="1.54.2" + +CDPATH="" cd -- "$(dirname -- "$0")/.." +BIN="$(pwd -P)"/bin + +mkdir -p "$BIN" + +# install golangci-lint bin/golangci-lint doesn't exist with the correct version +if ! "$BIN"/golangci-lint --version 2> /dev/null | grep -q "$GOLANGCI_LINT_VERSION"; then + GOBIN="$BIN" go install "github.com/golangci/golangci-lint/cmd/golangci-lint@v$GOLANGCI_LINT_VERSION" +fi + +MOD_DIRS="$(git ls-files '*go.mod' | xargs dirname | sort)" + +for dir in $MOD_DIRS; do + [ "$dir" = "example/newreposecretwithlibsodium" ] && continue + echo linting "$dir" + ( + cd "$dir" + # github actions output when running in an action + if [ -n "$GITHUB_ACTIONS" ]; then + "$BIN"/golangci-lint run --path-prefix "$dir" --out-format github-actions + else + "$BIN"/golangci-lint run --path-prefix "$dir" + fi + ) || FAILED=1 +done + +script/generate.sh --check || FAILED=1 + +if [ -n "$FAILED" ]; then + exit 1 +fi diff --git a/script/test.sh b/script/test.sh new file mode 100755 index 00000000000..23e6a0c8f84 --- /dev/null +++ b/script/test.sh @@ -0,0 +1,26 @@ +#!/bin/sh +#/ script/test.sh runs tests on each go module in go-github. Arguments are passed to each go test invocation. +#/ "-race -covermode atomic ./..." is used when no arguments are given. + +set -e + +CDPATH="" cd -- "$(dirname -- "$0")/.." + +if [ "$#" = "0" ]; then + set -- -race -covermode atomic ./... +fi + +MOD_DIRS="$(git ls-files '*go.mod' | xargs dirname | sort)" + +for dir in $MOD_DIRS; do + [ "$dir" = "example/newreposecretwithlibsodium" ] && continue + echo "testing $dir" + ( + cd "$dir" + go test "$@" + ) || FAILED=1 +done + +if [ -n "$FAILED" ]; then + exit 1 +fi