Skip to content

feat: add shellcheck#1250

Open
jceb wants to merge 1 commit intoalexellis:masterfrom
jceb:feat/add-shellcheck
Open

feat: add shellcheck#1250
jceb wants to merge 1 commit intoalexellis:masterfrom
jceb:feat/add-shellcheck

Conversation

@jceb
Copy link
Contributor

@jceb jceb commented Feb 12, 2026

Description

Add shellcheck, closes #1188

How Has This Been Tested?

If updating or adding a new CLI to arkade get, run:

go build && ./hack/test-tool.sh shellcheck

+ ./arkade get shellcheck --arch arm64 --os darwin --quiet
+ file /home/jceb/.arkade/bin/shellcheck
/home/jceb/.arkade/bin/shellcheck: Mach-O 64-bit arm64 executable, flags:<NOUNDEFS|DYLDLIN
K|TWOLEVEL|PIE>
+ rm /home/jceb/.arkade/bin/shellcheck
+ echo

+ ./arkade get shellcheck --arch x86_64 --os darwin --quiet
+ file /home/jceb/.arkade/bin/shellcheck
/home/jceb/.arkade/bin/shellcheck: Mach-O 64-bit x86_64 executable, flags:<NOUNDEFS|DYLDLI
NK|TWOLEVEL|PIE>
+ rm /home/jceb/.arkade/bin/shellcheck
+ echo

+ ./arkade get shellcheck --arch x86_64 --os linux --quiet
+ file /home/jceb/.arkade/bin/shellcheck
/home/jceb/.arkade/bin/shellcheck: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), st
atically linked, BuildID[sha1]=b26f8179194726f7313fd0bf5cf0f639bbde5023, stripped
+ rm /home/jceb/.arkade/bin/shellcheck
+ echo

+ ./arkade get shellcheck --arch aarch64 --os linux --quiet
+ file /home/jceb/.arkade/bin/shellcheck
/home/jceb/.arkade/bin/shellcheck: ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/
Linux), statically linked, BuildID[sha1]=176694d7ca08b43dd0fedf65f5db7c792f846bd6, for GNU
/Linux 3.7.0, stripped
+ rm /home/jceb/.arkade/bin/shellcheck
+ echo

+ ./arkade get shellcheck --arch x86_64 --os mingw --quiet
+ file /home/jceb/.arkade/bin/shellcheck.exe
/home/jceb/.arkade/bin/shellcheck.exe: PE32+ executable (console) x86-64, for MS Windows, 
9 sections
+ rm /home/jceb/.arkade/bin/shellcheck.exe
+ echo

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Documentation

  • I have updated the list of tools in README.md if (required) with ./arkade get --format markdown
  • I have updated the list of apps in README.md if (required) with ./arkade install --help

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have tested this on arm, or have added code to prevent deployment

@reviewfn

This comment has been minimized.

@jceb jceb force-pushed the feat/add-shellcheck branch 2 times, most recently from 598b87f to d334ffc Compare February 12, 2026 12:31
@reviewfn

This comment has been minimized.

@jceb jceb force-pushed the feat/add-shellcheck branch from d334ffc to 7e32393 Compare February 12, 2026 12:40
@reviewfn

This comment has been minimized.

@jceb jceb force-pushed the feat/add-shellcheck branch from 7e32393 to cccd030 Compare February 12, 2026 12:43
@reviewfn

This comment has been minimized.

Signed-off-by: Jan Christoph Ebersbach <[email protected]>
@jceb jceb force-pushed the feat/add-shellcheck branch from cccd030 to 593395b Compare February 12, 2026 12:47
@reviewfn
Copy link

reviewfn bot commented Feb 12, 2026

AI Pull Request Overview

Summary

  • Adds support for downloading shellcheck, a static analysis tool for shell scripts, via arkade get shellcheck
  • Updates README.md with the new tool listing and increments tool count to 192
  • Adds comprehensive test cases in get_test.go covering Windows, Linux, and macOS across multiple architectures
  • Implements URL template in tools.go for generating download URLs from shellcheck's GitHub releases

Approval rating (1-10)

8/10 - Solid implementation with good testing, but minor issues with version handling and description formatting.

Summary per file

Summary per file
File path Summary
README.md Added shellcheck to tools list and updated tool count from 191 to 192
pkg/get/get_test.go Added Test_DownloadShellcheck with test cases for multiple OS/arch combinations
pkg/get/tools.go Added shellcheck Tool definition with URLTemplate and binary configuration

Overall Assessment

The PR successfully adds shellcheck as a new downloadable tool in arkade, following established patterns for tool definitions and testing. The implementation covers key platforms and architectures with appropriate URL templates. However, there are concerns around version pinning and minor formatting issues that should be addressed for consistency and reliability.

Detailed Review

Detailed Review

README.md

  • The addition of shellcheck to the tools table is correct and the count update is accurate.
  • No issues with formatting or placement.

pkg/get/tools.go

  • Version Handling: The shellcheck tool definition lacks a VersionStrategy or explicit Version field. Other tools in the codebase (e.g., logcli) specify VersionStrategy: GitHubVersionStrategy to enable automatic version detection. Without this, the tool may fail to download if no default version is configured. Consider adding VersionStrategy: GitHubVersionStrategy or pinning to a specific version like Version: "v0.11.0" for reliability.
  • Description Formatting: The description has a trailing space: "A static analysis tool for shell scripts " - remove the extra space for consistency with other tool descriptions.
  • URLTemplate Logic: The template correctly handles different OS/arch combinations and matches shellcheck's release naming convention. The logic is sound and follows patterns used by other tools like websocat.
  • BinaryTemplate: Simple and correct - just "shellcheck" as the binary name.

pkg/get/get_test.go

  • Test Coverage: Good coverage of supported platforms (mingw, linux, darwin) and architectures (x86_64, aarch64/arm64). Includes Windows .zip and Unix .tar.gz formats.
  • Hardcoded Version: The test uses a const version = "v0.11.0" which matches the expected URLs. However, this should be verified against the tool's actual version resolution logic. If the tool uses GitHubVersionStrategy, the test should use the latest version rather than a hardcoded one.
  • Test Structure: Follows the same pattern as other tool tests (e.g., Test_DownloadCrush), with proper subtests for each OS/arch combination.
  • Potential Issue: The test assumes v0.11.0 is the current version, but without version strategy in the tool definition, this may not hold. Recommend checking if v0.11.0 is indeed the latest release from koalaman/shellcheck.

General Concerns

  • Security: Downloading from GitHub releases is standard practice for this codebase and poses no additional security risks.
  • Performance: No performance impact - static URL generation has negligible overhead.
  • Migration/Regressions: Low risk - addition of new tool doesn't affect existing functionality.
  • Consistency: The implementation is consistent with other tool additions, though the missing VersionStrategy is an inconsistency that should be resolved.

Recommendations

  1. Add VersionStrategy: GitHubVersionStrategy to the shellcheck tool definition to enable automatic version detection.
  2. Remove trailing space from the description.
  3. Verify that v0.11.0 is the latest shellcheck release and consider updating if newer versions are available.
  4. Run the full test suite to ensure no regressions in other tool downloads.

AI agent details.

Agent processing time: 32.532s
Environment preparation time: 5.978s
Total time from webhook: 42.959s

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.

[feat]: add shellcheck

1 participant