Skip to content

chore: Speed up linting#4071

Merged
gmlewis merged 1 commit intogoogle:masterfrom
alexandear-org:chore/improve-setup-custom-gcl
Mar 8, 2026
Merged

chore: Speed up linting#4071
gmlewis merged 1 commit intogoogle:masterfrom
alexandear-org:chore/improve-setup-custom-gcl

Conversation

@alexandear
Copy link
Copy Markdown
Contributor

@alexandear alexandear commented Mar 7, 2026

I propose parallelizing golangci-lint runs on modules. This can slightly speed up the lint step.

Lint step on my machine (without cache): before 60s, after 46s.

Before After Change
Wall clock 59.8s 45.7s -24%
CPU utilization 375% 522% +147%
User CPU 114.5s 118.3s ~same
System CPU 110.0s 120.5s ~same

Updates #4070

Before:

./bin/custom-gcl cache cleantime ./script/lint.sh
linting .
0 issues.
linting example
/Users/alexandear/src/github.com/google/go-github/example
0 issues.
linting otel
/Users/alexandear/src/github.com/google/go-github/otel
0 issues.
linting scrape
/Users/alexandear/src/github.com/google/go-github/scrape
0 issues.
linting tools
/Users/alexandear/src/github.com/google/go-github/tools
0 issues.
linting tools/check-structfield-settings
/Users/alexandear/src/github.com/google/go-github/tools/check-structfield-settings
0 issues.
linting tools/fmtpercentv
/Users/alexandear/src/github.com/google/go-github/tools/fmtpercentv
0 issues.
linting tools/sliceofpointers
/Users/alexandear/src/github.com/google/go-github/tools/sliceofpointers
0 issues.
linting tools/structfield
/Users/alexandear/src/github.com/google/go-github/tools/structfield
0 issues.
validating generated files
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/tools/metadata
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/example
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/example/newreposecretwithlibsodium
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/otel
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/scrape
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/tools
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/tools/check-structfield-settings
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/tools/fmtpercentv
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/tools/sliceofpointers
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.A7dIZ4R7N4/tools/structfield
./script/lint.sh  114.55s user 109.97s system 375% cpu 59.842 total

After:

git switch chore/improve-setup-custom-gcl
Switched to branch 'chore/improve-setup-custom-gcl'
Your branch is up to date with 'origin/chore/improve-setup-custom-gcl'../bin/custom-gcl cache cleantime ./script/lint.sh
linting .
linting example
/Users/alexandear/src/github.com/google/go-github/example
linting otel
/Users/alexandear/src/github.com/google/go-github/otel
linting scrape
/Users/alexandear/src/github.com/google/go-github/scrape
linting tools
/Users/alexandear/src/github.com/google/go-github/tools
linting tools/check-structfield-settings
/Users/alexandear/src/github.com/google/go-github/tools/check-structfield-settings
linting tools/fmtpercentv
/Users/alexandear/src/github.com/google/go-github/tools/fmtpercentv
linting tools/sliceofpointers
/Users/alexandear/src/github.com/google/go-github/tools/sliceofpointers
linting tools/structfield
/Users/alexandear/src/github.com/google/go-github/tools/structfield
0 issues.
0 issues.
0 issues.
0 issues.
0 issues.
0 issues.
0 issues.
0 issues.
0 issues.
validating generated files
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/tools/metadata
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/example
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/example/newreposecretwithlibsodium
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/otel
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/scrape
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/tools
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/tools/check-structfield-settings
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/tools/fmtpercentv
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/tools/sliceofpointers
/var/folders/q0/5_z6pvw574z1c0zcrr2xvk0r0000gn/T/tmp.LTjN6a1alw/tools/structfield
./script/lint.sh  118.34s user 120.52s system 522% cpu 45.716 total

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.65%. Comparing base (4f6e537) to head (3dadb4d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4071   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files         210      210           
  Lines       19416    19416           
=======================================
  Hits        18184    18184           
  Misses       1034     1034           
  Partials      198      198           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Very nice, @alexandear!
In looking at the "after" output, it is challenging to see what each "0 issues" refers to.
Instead of printing the "Linting blah/blah" BEFORE linting and then "0 issues" AFTER linting, would it be possible to not print anything before, and instead print the whole message after linting? For example "Linting blah/blah: 0 issues" all on a single line?

I'm thinking we don't want to modify golangci-lint itself, but maybe some bash magic could capture the info and capture the response then combine it together after it completes into a one-liner?

If not, no biggie, but that might make the AFTER text more useful... otherwise we may as well just hide it completely... or maybe we should just hide the "0 issues" since that's not terribly interesting.

Maybe you could artificially inject some linting issues in the various directories and experiment to see what the most useful output looks like?

@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Mar 7, 2026
@gmlewis
Copy link
Copy Markdown
Collaborator

gmlewis commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@munlicode munlicode left a comment

Choose a reason for hiding this comment

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

LGTM

@munlicode
Copy link
Copy Markdown
Contributor

munlicode commented Mar 8, 2026

Very nice, @alexandear! In looking at the "after" output, it is challenging to see what each "0 issues" refers to. Instead of printing the "Linting blah/blah" BEFORE linting and then "0 issues" AFTER linting, would it be possible to not print anything before, and instead print the whole message after linting? For example "Linting blah/blah: 0 issues" all on a single line?

I'm thinking we don't want to modify golangci-lint itself, but maybe some bash magic could capture the info and capture the response then combine it together after it completes into a one-liner?

If not, no biggie, but that might make the AFTER text more useful... otherwise we may as well just hide it completely... or maybe we should just hide the "0 issues" since that's not terribly interesting.

Maybe you could artificially inject some linting issues in the various directories and experiment to see what the most useful output looks like?

I have experimented on bash. Here show it looks:

Success

image

Error

image

Though, I have made considerable changes to script/lint.sh. I just found that it was complicated to read output, especially when everything is gray.

It can be added as separate PR to avoid over-complicating this one.

Lint Script

#!/bin/sh
#/ [ CHECK_GITHUB_OPENAPI=1 ] script/lint.sh runs linters and validates generated files.
#/ When CHECK_GITHUB is set, it validates that openapi_operations.yaml is consistent with the
#/ descriptions from github.com/github/rest-api-description.

set -e

CUSTOM_GCL="$(script/setup-custom-gcl.sh)"

CDPATH="" cd -- "$(dirname -- "$0")/.."

EXIT_CODE=0

# Colors & Formatting
GREEN='\033[0;32m'
RED='\033[0;31m'
YELLOW='\033[0;33m'
BOLD='\033[1m'
NC='\033[0m'

fail() {
  EXIT_CODE=1
}

MOD_DIRS="$(git ls-files '*go.mod' | xargs dirname | sort -u)"

# Number of module lint jobs to run concurrently.
# Override with LINT_JOBS, otherwise use detected CPU count.
: "${LINT_JOBS:=$(getconf _NPROCESSORS_ONLN 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 4)}"

LINT_DIRS="$(printf '%s\n' "$MOD_DIRS" | grep -v '^example/newreposecretwithlibsodium$')"

FAILED_COUNT=0
LINT_FAILED=0
RUNNING=0
PIDS=""
DIRS_IN_FLIGHT=""

LOG_DIR="$(mktemp -d)"
trap 'rm -rf "$LOG_DIR"' EXIT

# --- Helper Functions ---
print_header() {
    printf "${BOLD}%s${NC}\n\n" "$1"
}


wait_pids() {
  i=1
  for pid in $PIDS; do
    # Identify the directory for this PID
    dir=$(echo "$DIRS_IN_FLIGHT" | awk -v i="$i" '{print $i}')
    log_file="$LOG_DIR/$(echo "$dir" | tr '/' '_').log"
    
    if wait "$pid"; then
      printf "${GREEN}✔ %-40s [ PASS ]${NC}\n" "$dir"
    else
      printf "${RED}✘ %-40s [ FAIL ]${NC}\n" "$dir"
      if [ -f "$log_file" ]; then
        sed 's/^/    /' "$log_file"
      fi
      FAILED_COUNT=$((FAILED_COUNT + 1))
      fail
    fi
    i=$((i + 1))
  done
  PIDS=""
  DIRS_IN_FLIGHT=""
  RUNNING=0
}

print_header "Linting modules"

for dir in $LINT_DIRS; do
  log_file="$LOG_DIR/$(echo "$dir" | tr '/' '_').log"

  # Run the linter in the background and redirect output to a log file
  (cd "$dir" && "$CUSTOM_GCL" run --color always > "$log_file" 2>&1) &

  PIDS="$PIDS $!"
  DIRS_IN_FLIGHT="$DIRS_IN_FLIGHT $dir"
  RUNNING=$((RUNNING + 1))

  if [ "$RUNNING" -ge "$LINT_JOBS" ]; then
    wait_pids
  fi
done

wait_pids

if [ -n "$CHECK_GITHUB_OPENAPI" ]; then
  print_header "Validating openapi_operations.yaml"
  if script/metadata.sh update-openapi --validate; then
      printf "${GREEN}✔ openapi_operations.yaml is valid${NC}\n"
  else
      printf "${RED}✘ openapi_operations.yaml validation failed${NC}\n"
      fail
  fi
fi

print_header "Validating generated files"
if script/generate.sh --check; then
    printf "${GREEN}✔ Generated files are up to date${NC}\n"
else
    printf "${RED}✘ Generated files out of sync${NC}\n"
    fail
fi

# --- Final Summary ---
printf -- "----------------------------\n"
SUMMARY_COLOR="$GREEN"
if [ "$FAILED_COUNT" -gt 0 ]; then
    SUMMARY_COLOR="$RED"
fi

printf "%bLinting: issues found in %d module directories.%b\n" "$SUMMARY_COLOR" "$FAILED_COUNT" "$NC"
printf -- "--------------------------------------------\n"

exit "$EXIT_CODE"

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Mar 8, 2026
Copy link
Copy Markdown
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Wow, @munlicode - that looks fantastic! Thank you!

OK, great! I'm going to go ahead and merge this one then you can create a new PR to make the output beautiful and super-easy to read and understand.

LGTM.
Merging.

@gmlewis gmlewis merged commit 16e8203 into google:master Mar 8, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants