Skip to content

fix: add linter for PR-only changes in samples-go #198

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.

Sign up for GitHub

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

Open
SaketV8 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: add linter for PR-only changes in samples-go #198

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
007b32b
fix: add linter for PR-only changes in samples-go
SaketV8 Jan 18, 2026
25bd37b
fix: removed unused commented code & fixed some edge cases
SaketV8 Jan 20, 2026
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
91 changes: 66 additions & 25 deletions .github/workflows/golangci-lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,80 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }}
cancel-in-progress: true


jobs:
detect-go-projects:
name: project-changes
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}

steps:
# Checkout full history so git diff works correctly
- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Build matrix of changed Go projects
id: set-matrix
run: |
# Ensure base branch is available locally for diff
echo "Step 0: prepare diff base"
if [ "${{ github.event_name }}" = "pull_request" ]; then
git fetch origin ${{ github.base_ref }}
diff_base="origin/${{ github.base_ref }}"
else
diff_base="HEAD~1"

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Using HEAD1 for push events may fail on the initial commit or when the repository has shallow clones from other workflows. Consider using a more robust approach, such as comparing against the specific commit SHA from the before field in the push event payload using github.event.before, or handle the case where HEAD1 doesn't exist.

Suggested change
diff_base="HEAD~1"
# For push events, use the commit SHA from the event payload when available.
if [ "${{ github.event.before }}" != "0000000000000000000000000000000000000000" ]; then
diff_base="${{ github.event.before }}"
else
# Initial commit: diff against an empty tree.
diff_base="$(git hash-object -t tree /dev/null)"
fi

Copilot uses AI. Check for mistakes.

@SaketV8 SaketV8 Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed this edge case

fi

echo "Step 1: get changed files"
files=$(git diff --name-only "$diff_base"...HEAD)

# echo "Step 1: get changed files"
# files=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Remove commented-out code. These lines appear to be leftover from development and should be deleted to keep the workflow clean and maintainable.

Copilot uses AI. Check for mistakes.

@SaketV8 SaketV8 Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed


echo "Step 2: get top-level directories"
dirs=$(echo "$files" | cut -d/ -f1 | sort -u)
Comment on lines +52 to +56

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

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

The script does not handle the case where no files have changed. If the files variable is empty, the subsequent commands (cut, sort) will still execute but may produce unexpected results. Consider adding a check after line 46 to handle the empty case explicitly and short-circuit to an empty matrix early.

Copilot uses AI. Check for mistakes.

@SaketV8 SaketV8 Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested this in my fork, and the no-file-change case is already handled, an empty diff results in an empty matrix '[ ]', which correctly prevents the lint job from running, this matches the intended workflow behavior.

Here's the screenshot from my forked version:

image


echo "Step 3: filter go projects"
projects=()
for dir in $dirs; do
if [ -f "$dir/go.mod" ]; then
# projects+=("\"$dir\"")

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Remove commented-out code. This line appears to be leftover from testing different approaches and should be deleted.

Suggested change
# projects+=("\"$dir\"")

Copilot uses AI. Check for mistakes.

@SaketV8 SaketV8 Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

projects+=("$dir")
fi
done

echo "Step 4: build matrix json"
# matrix=$(printf '%s\n' "${projects[@]}" | jq -R . | jq -cs .)

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Remove commented-out code. This line appears to be an alternative implementation using jq and should be deleted if it's no longer needed.

Suggested change
# matrix=$(printf '%s\n' "${projects[@]}" | jq -R . | jq -cs .)

Copilot uses AI. Check for mistakes.

@SaketV8 SaketV8 Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

if [ ${#projects[@]} -eq 0 ]; then
matrix="[]"
else
joined=$(printf '"%s",' "${projects[@]}")
joined="[${joined%,}]"
matrix="$joined"
fi

echo "Step 5: final changed go projects :"
echo "$matrix"

echo "matrix={\"working-directory\":$matrix}" >> "$GITHUB_OUTPUT"
Comment on lines +35 to +78

Copilot AI Jan 18, 2026

Copy link

Choose a reason for hiding this comment

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

The script does not handle the case where git diff might fail or produce unexpected output. Add error handling with set -e at the beginning of the script to exit on errors, or add explicit error checks after critical commands to ensure the workflow fails gracefully if something goes wrong.

Copilot uses AI. Check for mistakes.

@SaketV8 SaketV8 Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

github workflow by default use the bash -e {0}, so set -e is active

here the doc link - https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#defaultsrunshell


golangci:
name: lint
needs: detect-go-projects
# Skip job if no Go projects were modified
if: ${{ needs.detect-go-projects.outputs.matrix != '{"working-directory":[]}' }}
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
working-directory:
- echo-mysql
- echo-sql
- fasthttp-postgres
- gin-mongo
- gin-redis
- go-grpc
- go-jwt
- go-twilio
- graphql-sql
- http-pokeapi
- mux-elasticsearch
- mux-mysql
- mux-sql
- S3-Keploy
- sse-svelte
- users-profile
- book-store-inventory

matrix: ${{ fromJson(needs.detect-go-projects.outputs.matrix) }}

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: '1.23.4'
cache: false
go-version: "1.23.4"
cache: true
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
Expand All @@ -62,4 +103,4 @@ jobs:
working-directory: ${{matrix.working-directory}}

# Optional: The mode to install golangci-lint. It can be 'binary' or 'goinstall'.
install-mode: "goinstall"
install-mode: "goinstall"
Loading