Conversation
Extract all inline strings to constants at the top of main.rs for improved
maintainability and consistency:
- Path components: WRAPPER_DIR_NAME, WRAPPER_SCRIPT_NAME, etc.
- User messages: MSG_BYPASS_INIT
- Error messages: ERR_FAILED_EXECUTE_GIT, ERR_NOT_GIT_REPO, etc.
- File templates: HOOK_SCRIPT_TEMPLATE, SAMPLE_PRE_COMMIT_CONTENT, etc.
Production code uses constants (format!("{}: {}", ERR_*, error)) while
tests use literal values to detect accidental constant changes.
Add project-level Claude Code configuration with: - Permissions for allowed bash commands (cargo, git, shellcheck, etc.) - MCP tool permissions for GitHub integration - Deny rules for docs and target directories
Add comprehensive rustdoc documentation to all constants: - Error constants: Document all 24 ERR_* constants with clear descriptions - User messages: Document MSG_BYPASS_INIT for bypass mode - Templates: Document HOOK_SCRIPT_TEMPLATE, SAMPLE_PRE_COMMIT_CONTENT, and GITIGNORE_CONTENT Also add "Error: " prefix to ERR_UNABLE_RESOLVE_PATH and ERR_UNABLE_RESOLVE_PARENT for consistency with other error messages.
- Add install.sh with platform detection, checksum verification, and PATH configuration - Add uninstall.sh for clean removal from common installation locations - Include comprehensive implementation guide covering curl/bash patterns, security considerations, and best practices for Rust projects - Scripts support Linux (musl), macOS (darwin), with helpful Windows guidance - Installation validates prerequisites, detects platform, verifies checksums - Uninstallation handles multiple common locations with clear user feedback Scripts tested on Ubuntu 24.04, Fedora 42, and macOS (see .docs guide for details)
CI Enhancements: - Add installer-test job validating install.sh and uninstall.sh - Test scripts with shellcheck for syntax validation - Verify platform detection on Ubuntu 24.04, Fedora 42, macOS (native) - Test installation/uninstallation workflows using Docker containers - Integrate installer-test into ci-success dependencies Documentation Updates: - Add curl/bash as primary installation method in README - Include Quick Start section with complete workflow - Add "Why Samoyed?" feature highlights - Enhance Usage section with directory structure and examples - Improve Configuration section with real-world examples - Restructure content: Quick Start → Install → Usage → Background - Document all 4 GitHub workflows (was 2): CI, Release PR, Release Tag, Release Pipeline - Add installer script testing guide and troubleshooting Configuration: - Whitelist .docs/02-bash-curl-install.md for Claude Code access All installer tests passing on Ubuntu, Fedora, and macOS
- Add crates.io version badge with clickable link - Change 'minimal' to 'minimalist' for consistency - Clarify '~1000 lines of code' (not total lines) - Disable MD040 markdownlint rule for code blocks without language tags
Windows Build Fix: - Add #[cfg(unix)] to ERR_FAILED_GET_METADATA constant - Add #[cfg(unix)] to ERR_FAILED_SET_PERMISSIONS constant - These constants are only used in Unix permission code blocks - Prevents dead_code warnings on Windows (treated as errors with -D warnings) macOS Installer Test Fix: - Extract all function dependencies (colors, logging functions) - Previously only extracted detect_platform() which called undefined functions - Now extracts: color constants, info(), warn(), error(), die(), detect_platform() - Writes to temp file and sources it to avoid broken pipe errors - Fixes exit code 127 (command not found) error Fixes #146 CI failures
Contributor
📊 Coverage Report
View detailed reportGenerated by cargo-tarpaulin |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request significantly enhances the CI/CD documentation and pipeline for Samoyed, introducing comprehensive installer script testing and automation while adding curl/bash installation capabilities for the project.
- Adds complete curl/bash installation with
install.shanduninstall.shscripts - Introduces installer script testing in CI with cross-platform validation
- Extensively refactors Rust code for maintainability using constants for error messages and configuration
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
install.sh |
New curl/bash installer script with platform detection, checksum verification, and PATH configuration |
uninstall.sh |
New uninstaller script that removes binaries from common installation locations |
src/main.rs |
Major refactoring to use constants for error messages, file names, and script templates |
README.md |
Updated with installation instructions and comprehensive usage documentation |
.github/workflows/ci.yml |
Added installer script testing across Ubuntu, Fedora, and macOS platforms |
.github/workflows/README.md |
Extensively expanded CI/CD documentation with detailed workflow descriptions |
.docs/02-bash-curl-install.md |
Comprehensive guide on implementing curl/bash installation patterns |
.markdownlint.json |
Added MD040 rule exception for code blocks without language specification |
.claude/settings.json |
New configuration file specifying environment variables and permissions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+762
to
+767
| assert_eq!( | ||
| content, | ||
| r#"#!/usr/bin/env sh | ||
| . "$(dirname "$0")/samoyed" | ||
| "# | ||
| ); |
There was a problem hiding this comment.
The test assertion uses a hardcoded string literal instead of the HOOK_SCRIPT_TEMPLATE constant. This creates maintenance overhead if the template changes. Consider using assert_eq!(content, HOOK_SCRIPT_TEMPLATE); to maintain consistency with the actual implementation.
Comment on lines
+801
to
+805
| r#"#!/usr/bin/env sh | ||
| # Add your pre-commit checks here. For example: | ||
| # echo "Running Samoyed sample pre-commit" | ||
| # exit 0 | ||
| "# |
There was a problem hiding this comment.
The test assertion uses a hardcoded string literal instead of the SAMPLE_PRE_COMMIT_CONTENT constant. This creates maintenance overhead if the template changes. Consider using assert_eq!(content, SAMPLE_PRE_COMMIT_CONTENT); to maintain consistency with the actual implementation.
Suggested change
| r#"#!/usr/bin/env sh | |
| # Add your pre-commit checks here. For example: | |
| # echo "Running Samoyed sample pre-commit" | |
| # exit 0 | |
| "# | |
| SAMPLE_PRE_COMMIT_CONTENT |
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request significantly enhances the CI/CD documentation and pipeline for the project, with a strong focus on automation, security, and installer script validation. The main improvements include a detailed overhaul of the CI/CD workflow documentation, the addition of comprehensive installer script testing in CI, and updates to best practices and local testing instructions. These changes make the release process fully automated and improve reliability across platforms.
CI/CD Pipeline Documentation and Process Overhaul:
.github/workflows/README.mdto provide a detailed, step-by-step explanation of the CI/CD pipeline, including clear descriptions of each workflow (CI, release-pr, release-plz, release), their triggers, and the automated release process. The documentation now features workflow diagrams, explicit job breakdowns, and best practices for contributors. [1] [2] [3] [4] [5] [6] [7] [8]Installer Script Testing and Validation:
installer-testjob to.github/workflows/ci.ymlthat validatesinstall.shanduninstall.shwith shellcheck, tests platform detection, and verifies installation/uninstallation workflows on Ubuntu, Fedora (via Docker), and macOS. This ensures installer scripts work reliably across supported platforms.installer-testto pass before marking CI as successful, preventing releases if installer scripts fail.Contributor Guidance and Best Practices:
Security and Permissions:
.claude/settings.jsonfile specifying environment variables and fine-grained permissions for automated tools, further tightening security and access control.