-
Notifications
You must be signed in to change notification settings - Fork 14
chore: add devcontainer and Taskfile workflow for multi-platform builds #16
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a devcontainer and setup script for a Go dev environment, a Taskfile for cross-platform build/package workflows, README updates for building and shell completion, and expands .gitignore to include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (4)
.devcontainer/devcontainer.json (1)
9-13: Redundant Go feature.The base image
mcr.microsoft.com/devcontainers/go:2-1.25-trixiealready includes the Go SDK. Addingghcr.io/devcontainers/features/go:1may cause version conflicts or is simply unnecessary overhead.🔎 Proposed fix
"features": { - "ghcr.io/devcontainers/features/go:1": {}, "ghcr.io/guiyomh/features/golangci-lint:0": {}, "ghcr.io/guiyomh/features/goreleaser:0": {} },.devcontainer/setup.sh (1)
10-10: Potential failure if.codexdirectory doesn't exist.If the volume mount hasn't created
/home/vscode/.codexyet, thischowncommand will fail. Consider adding a directory creation guard.🔎 Proposed fix
+mkdir -p /home/vscode/.codex sudo chown -R ${UID}:${UID} /home/vscode/.codexTaskfile.yml (2)
32-87: Consider reducing duplication with Taskfile's dynamic features.The build tasks share nearly identical structure. Taskfile supports
forloops or dynamic task generation that could consolidate these into a single parameterized task, improving maintainability.This is optional since the current explicit approach is clear and functional.
3-6: Consider embedding version info in LDFLAGS.Adding
-Xlinker flags to embed version, commit, and build date into the binary helps with debugging and user support (e.g.,hapi --versionshowing full build info).🔎 Example enhancement
env: DIST_DIR: dist BINARY: hapi VERSION: sh: git describe --tags --abbrev=0 2>/dev/null || echo "dev" COMMIT: sh: git rev-parse --short HEAD LDFLAGS: -s -w -X main.version={{.VERSION}} -X main.commit={{.COMMIT}}This requires corresponding variables in
main.goto receive these values.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.devcontainer/devcontainer.json(1 hunks).devcontainer/setup.sh(1 hunks).gitignore(1 hunks)README.md(2 hunks)Taskfile.yml(1 hunks)
🔇 Additional comments (3)
.gitignore (1)
1-4: LGTM!The additions of
bin/anddist/correctly align with the new Taskfile-based build workflow, preventing build artifacts from being committed.README.md (1)
48-72: Good documentation for the build workflow.The new "Building from source" section clearly explains both the Taskfile approach and manual
go buildcommands, with appropriate artifact locations documented..devcontainer/devcontainer.json (1)
6-6: The image tag2-1.25-trixieis valid and current. Go 1.25.5 was released on December 2, 2025, making this a stable and appropriate choice for a dev container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Taskfile.yml (1)
118-134: Fix Windows package tasks to reference .exe files.The Windows package tasks attempt to tar
${BINARY}, but the Windows build tasks output${BINARY}.exe. This mismatch will cause tar to fail with "file not found" errors.🔎 Proposed fix
package.windows-386: desc: Package the Windows 386 binary into a tarball. deps: [build.windows-386] cmds: - - sh -c 'version=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD); tar czf ${DIST_DIR}/${BINARY}-$version-windows-386.tar.gz -C ${DIST_DIR}/windows-386 ${BINARY}' + - sh -c 'version=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD); tar czf ${DIST_DIR}/${BINARY}-$version-windows-386.tar.gz -C ${DIST_DIR}/windows-386 ${BINARY}.exe' package.windows-amd64: desc: Package the Windows amd64 binary into a tarball. deps: [build.windows-amd64] cmds: - - sh -c 'version=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD); tar czf ${DIST_DIR}/${BINARY}-$version-windows-amd64.tar.gz -C ${DIST_DIR}/windows-amd64 ${BINARY}' + - sh -c 'version=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD); tar czf ${DIST_DIR}/${BINARY}-$version-windows-amd64.tar.gz -C ${DIST_DIR}/windows-amd64 ${BINARY}.exe' package.windows-arm64: desc: Package the Windows arm64 binary into a tarball. deps: [build.windows-arm64] cmds: - - sh -c 'version=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD); tar czf ${DIST_DIR}/${BINARY}-$version-windows-arm64.tar.gz -C ${DIST_DIR}/windows-arm64 ${BINARY}' + - sh -c 'version=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD); tar czf ${DIST_DIR}/${BINARY}-$version-windows-arm64.tar.gz -C ${DIST_DIR}/windows-arm64 ${BINARY}.exe'
🧹 Nitpick comments (2)
.devcontainer/setup.sh (2)
10-10: Verify .codex directory exists or add defensive check.The script assumes
${HOME}/.codexexists before changing ownership. If this directory is not guaranteed to exist (via volume mount or prior creation), the command will fail and halt the script due toset -e.Consider adding a defensive check:
+mkdir -p ${HOME}/.codex sudo chown -R ${UID}:${UID} ${HOME}/.codex
10-10: Consider fallback for ${UID} variable.The
${UID}variable may not be set in all container environments. While it's typically available in Bash, consider using$(id -u):$(id -g)for better portability.🔎 Proposed refactor
Add at the top of the script after line 2:
set -euo pipefail +UID="${UID:-$(id -u)}" +GID="${GID:-$(id -g)}"Then update the chown commands to use
${UID}:${GID}or keep as-is if you prefer.Also applies to: 13-13, 19-19, 23-23, 27-27
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.devcontainer/devcontainer.json(1 hunks).devcontainer/setup.sh(1 hunks)README.md(2 hunks)Taskfile.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- .devcontainer/devcontainer.json
🔇 Additional comments (8)
.devcontainer/setup.sh (2)
1-8: LGTM! Previous issue resolved.The shebang, strict mode flags, and Task CLI installation logic are correct. The
-yflag is now present on line 7, resolving the previous concern about non-interactive hangs.
12-16: LGTM! Previous issue resolved.The file permission logic now correctly preserves execute bits on shell scripts by excluding
*.shfiles from the 644 permission set and explicitly setting them to 755. This resolves the previous concern about breaking executable scripts.Taskfile.yml (6)
1-6: LGTM!The Taskfile version and environment variable definitions are correct. The LDFLAGS
-s -wappropriately strips debug information for smaller release binaries.
8-30: LGTM!The default, clean, ensure, and build tasks are well-structured with appropriate descriptions, dependencies, and commands.
32-66: LGTM!The Linux and macOS build tasks correctly specify GOOS/GOARCH environment variables and output paths for their respective platforms.
67-86: LGTM! Previous issue resolved.The Windows build tasks now correctly output binaries with the
.exeextension (lines 72, 79, 86), resolving the previous concern about extensionless Windows binaries.
88-116: LGTM! Previous issue resolved.The Linux and macOS package tasks correctly use the git describe fallback pattern and reference the correct binary names.
136-157: LGTM! Previous issue resolved.The aggregate tasks (packages, checksums, release) are well-structured with proper dependency chains. The checksums task includes the git describe fallback, resolving the previous concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.devcontainer/setup.sh (2)
18-27: LGTM!The Git configuration setup now correctly creates
.git-credentialsas a file rather than a directory. Previous issue has been resolved.Optional: Remove redundant -R flag on line 27
The
-Rflag on line 27 is unnecessary since.git-credentialsis a file/symlink, not a directory:-sudo chown -R ${UID}:${UID} ${HOME}/.git-credentials +sudo chown ${UID}:${UID} ${HOME}/.git-credentials
29-29: Consider using--replace-allto prevent duplicate entries.Using
--addcan create duplicatesafe.directoryentries if the container is rebuilt or this script runs multiple times. Consider using--replace-allto ensure a single entry.🔎 Suggested improvement
-git config --global --add safe.directory ${PWD} +git config --global --replace-all safe.directory ${PWD}Alternatively, check before adding:
git config --global --get-all safe.directory | grep -qF "${PWD}" || \ git config --global --add safe.directory ${PWD}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.devcontainer/setup.sh(1 hunks)Taskfile.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Taskfile.yml
🔇 Additional comments (4)
.devcontainer/setup.sh (4)
1-2: LGTM!Excellent use of strict error handling with
set -euo pipefailto catch errors early.
4-8: LGTM!The Task CLI installation now includes the
-yflag for non-interactive execution. Previous issue has been resolved.
10-10: Ensure.codexdirectory exists before chown.The script will fail if
${HOME}/.codexdoesn't exist (due toset -e). If this directory is not guaranteed to be created before this script runs, consider creating it first.🔎 Suggested safeguard
+mkdir -p ${HOME}/.codex sudo chown -R ${UID}:${UID} ${HOME}/.codexPlease verify whether the devcontainer or another setup step creates
.codexbefore this script executes.
12-16: LGTM!The permission fixes now correctly preserve executable permissions on shell scripts while setting standard permissions on other files. Previous issue has been resolved.
#15
Introduces the devcontainer and Taskfile automation so contributors get a reproducible VS Code environment and can build binaries for each supported platform without manual setup.
Automation and reproducible workspace
.devcontainer/directory with VS Code settings, recommended extensions, and tooling (Go SDK, Task CLI, packaging tools) so everyone starts with the same environment.Taskfile.ymlwith tasks for cleaning (task clean), local builds (task build), and multi-platform releases (task release) that leverage goreleaser and the project’s configured variables.Commits
Testing:
Not run (configuration-only change).
please execute
task cleanandtask releasein the container/host to validate, for it u can installTask CLIor use the devcontainer terminal, then run:Example
vscode ➜ /workspaces/api-cli (main) $ task clean task: [clean] rm -rf ${DIST_DIR}vscode ➜ /workspaces/api-cli (main) $ tree ./dist/ ./dist/ ├── darwin-amd64 │ └── hapi ├── darwin-arm64 │ └── hapi ├── hapi-v0.1.12-checksums.sha256 ├── hapi-v0.1.12-darwin-amd64.tar.gz ├── hapi-v0.1.12-darwin-arm64.tar.gz ├── hapi-v0.1.12-linux-386.tar.gz ├── hapi-v0.1.12-linux-amd64.tar.gz ├── hapi-v0.1.12-linux-arm64.tar.gz ├── hapi-v0.1.12-windows-386.tar.gz ├── hapi-v0.1.12-windows-amd64.tar.gz ├── hapi-v0.1.12-windows-arm64.tar.gz ├── linux-386 │ └── hapi ├── linux-amd64 │ └── hapi ├── linux-arm64 │ └── hapi ├── windows-386 │ └── hapi.exe ├── windows-amd64 │ └── hapi.exe └── windows-arm64 └── hapi.exe 9 directories, 17 filesSummary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.