Add selectable repo flavours and release workflow#9
Conversation
Reviewer's GuideExtends the project template with a selectable “flavour” option to generate either a library or binary crate, implements conditional templating and release workflows for the app flavour, and updates documentation and tests to align with these changes. Class diagram for template file generation based on flavourclassDiagram
class TemplateEngine {
+generate_files(flavour)
}
class AppFlavour {
+main.rs
+release.yml
}
class LibFlavour {
+lib.rs
}
TemplateEngine --> AppFlavour : if flavour == app
TemplateEngine --> LibFlavour : if flavour == lib
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe project template now supports a configurable "flavour" option, allowing generation of either a library or application. Source files and CI workflows are conditionally rendered based on this selection. Tests and documentation were updated to reflect the new flexibility, and a cross-platform release workflow was added for application projects. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Copier
participant TemplateEngine
participant GitHubActions
User->>Copier: Selects project flavour (lib or app)
Copier->>TemplateEngine: Render source files and workflows based on flavour
TemplateEngine-->>Copier: Generates main.rs/lib.rs and workflows
Copier-->>User: Outputs project files
alt If flavour == app
GitHubActions->>GitHubActions: On tag push, run release workflow
GitHubActions->>GitHubActions: Build binaries (matrix)
GitHubActions->>GitHubActions: Upload release assets
end
Poem
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Refactor the release workflow template to reduce duplication in the matrix (e.g. by iterating over a target list or using YAML anchors) for easier maintenance.
- The Jinja conditional in the workflow filename may produce an empty or invalid path for the 'lib' flavour—consider separating the app-specific workflow into its own template or moving the condition inside the file.
- Add a test to verify that the release workflow file exists only for the 'app' flavour and is omitted when 'lib' is selected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Refactor the release workflow template to reduce duplication in the matrix (e.g. by iterating over a target list or using YAML anchors) for easier maintenance.
- The Jinja conditional in the workflow filename may produce an empty or invalid path for the 'lib' flavour—consider separating the app-specific workflow into its own template or moving the condition inside the file.
- Add a test to verify that the release workflow file exists only for the 'app' flavour and is omitted when 'lib' is selected.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)copier.yaml(1 hunks)template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja(1 hunks)template/src/main.rs.jinja(0 hunks)template/src/{% if flavour == 'app' %}main.rs{% else %}lib.rs{% endif %}.jinja(1 hunks)tests/test_template.py(2 hunks)
💤 Files with no reviewable changes (1)
- template/src/main.rs.jinja
🔇 Additional comments (6)
copier.yaml (1)
25-31: LGTM! Well-structured configuration option.The
flavourconfiguration is clearly defined with appropriate choices, sensible default, and helpful description. This follows Copier best practices consistently.tests/test_template.py (1)
24-24: Correct update for default flavour behaviour.The change from checking
main.rstolib.rscorrectly reflects that the default flavour is now "lib".README.md (2)
18-19: Clear documentation of conditional starter code.The documentation accurately describes the new flavour-based conditional generation of either binary or library entry points.
22-23: Good addition documenting the release workflow.The documentation correctly notes that the release workflow is only included for the app flavour, which helps users understand when to expect this functionality.
template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja (2)
15-57: Verify cross-compilation support for all target platforms.The workflow includes many cross-compilation targets but doesn't verify that cross-compilation actually works for all of them. Some targets may require additional setup or may not be supported by the cross tool.
#!/bin/bash # Description: Check which Rust targets are supported by the cross tool # Expected: List of supported targets to verify the workflow matrix # Check if cross supports all the targets mentioned in the workflow TARGETS=( "x86_64-unknown-linux-gnu" "aarch64-unknown-linux-gnu" "x86_64-pc-windows-msvc" "aarch64-pc-windows-msvc" "x86_64-apple-darwin" "aarch64-apple-darwin" "x86_64-unknown-freebsd" "aarch64-unknown-freebsd" "x86_64-unknown-openbsd" "aarch64-unknown-openbsd" ) echo "Checking cross-rs/cross repository for supported targets..." # Get the supported targets from cross documentation curl -s https://api.github.com/repos/cross-rs/cross/contents/docker | jq -r '.[].name' | grep -E '^Dockerfile\.' | sed 's/^Dockerfile\.//' | sort > supported_targets.txt echo "Supported targets by cross:" cat supported_targets.txt echo -e "\nChecking workflow targets against supported targets:" for target in "${TARGETS[@]}"; do if grep -q "^$target$" supported_targets.txt; then echo "✓ $target - SUPPORTED" else echo "✗ $target - NOT FOUND" fi done
42-57: Verify the necessity of FreeBSD and OpenBSD targets.Whilst comprehensive platform coverage is commendable, FreeBSD and OpenBSD targets may have limited usage and could complicate maintenance. Consider whether these targets are essential for most users or if they should be optional.
What is the current popularity and adoption rate of FreeBSD and OpenBSD for Rust applications in 2024?
| {%- if flavour == "app" -%} | ||
| fn main() { | ||
| println!("Hello from {{ project_name }}!"); | ||
| } | ||
| {%- else -%} | ||
| /// Returns a greeting for the library. | ||
| #[must_use] | ||
| pub fn greet() -> &'static str { | ||
| "Hello from {{ project_name }}!" | ||
| } | ||
| {%- endif -%} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Well-implemented conditional template with minor test gap.
The Jinja2 conditional logic cleanly separates app and library implementations. The use of #[must_use] on the library function is good practice.
However, consider adding a test for the library function to ensure it works correctly:
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_greet() {
let greeting = greet();
assert!(greeting.contains("{{ project_name }}"));
}
}🤖 Prompt for AI Agents
In template/src/lib.rs.jinja lines 1 to 11, add a test module with a test
function for the greet() function to verify it returns the expected greeting
string containing the project name. Implement a #[cfg(test)] mod tests with a
#[test] fn test_greet() that calls greet() and asserts the returned string
contains "{{ project_name }}". This will ensure the library function is properly
tested.
| - name: Prepare artifact | ||
| run: | | ||
| mkdir -p artifacts/${{ matrix.os }}-${{ matrix.arch }} | ||
| cp target/${{ matrix.target }}/release/${{ env.REPO_NAME }}${{ matrix.ext }} \ | ||
| artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} | ||
| sha256sum artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} > \ | ||
| artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}.sha256 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify artifact preparation with error handling.
The shell commands for preparing artifacts are complex and lack error handling. Consider simplifying or adding proper error checks:
- name: Prepare artifact
run: |
+ set -euo pipefail
mkdir -p artifacts/${{ matrix.os }}-${{ matrix.arch }}
cp target/${{ matrix.target }}/release/${{ env.REPO_NAME }}${{ matrix.ext }} \
artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}
+ cd artifacts/${{ matrix.os }}-${{ matrix.arch }}
- sha256sum artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} > \
- artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}.sha256
+ sha256sum ${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} > \
+ ${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}.sha256📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Prepare artifact | |
| run: | | |
| mkdir -p artifacts/${{ matrix.os }}-${{ matrix.arch }} | |
| cp target/${{ matrix.target }}/release/${{ env.REPO_NAME }}${{ matrix.ext }} \ | |
| artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} | |
| sha256sum artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} > \ | |
| artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}.sha256 | |
| - name: Prepare artifact | |
| run: | | |
| set -euo pipefail | |
| mkdir -p artifacts/${{ matrix.os }}-${{ matrix.arch }} | |
| cp target/${{ matrix.target }}/release/${{ env.REPO_NAME }}${{ matrix.ext }} \ | |
| artifacts/${{ matrix.os }}-${{ matrix.arch }}/${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} | |
| cd artifacts/${{ matrix.os }}-${{ matrix.arch }} | |
| sha256sum ${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }} > \ | |
| ${{ env.REPO_NAME }}-${{ matrix.os }}-${{ matrix.arch }}${{ matrix.ext }}.sha256 |
🤖 Prompt for AI Agents
In template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif
%}.jinja around lines 84 to 90, the shell commands for preparing artifacts are
complex and lack error handling. Simplify the commands by combining steps where
possible and add error handling by using set -e or checking command success
after each step to ensure the script stops on failure. This will make the
artifact preparation more robust and easier to maintain.
| - run: | | ||
| for dir in artifacts/${{ env.REPO_NAME }}-*; do | ||
| for file in "$dir"/*; do | ||
| gh release upload "${{ github.ref_name }}" "$file" | ||
| done | ||
| done |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to release upload loop.
The shell loop for uploading release assets lacks error handling and could fail silently:
- run: |
+ run: |
+ set -euo pipefail
for dir in artifacts/${{ env.REPO_NAME }}-*; do
+ if [[ ! -d "$dir" ]]; then
+ echo "Warning: No artifacts found in $dir"
+ continue
+ fi
for file in "$dir"/*; do
+ if [[ -f "$file" ]]; then
gh release upload "${{ github.ref_name }}" "$file"
+ fi
done
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - run: | | |
| for dir in artifacts/${{ env.REPO_NAME }}-*; do | |
| for file in "$dir"/*; do | |
| gh release upload "${{ github.ref_name }}" "$file" | |
| done | |
| done | |
| - run: | | |
| set -euo pipefail | |
| for dir in artifacts/${{ env.REPO_NAME }}-*; do | |
| if [[ ! -d "$dir" ]]; then | |
| echo "Warning: No artifacts found in $dir" | |
| continue | |
| fi | |
| for file in "$dir"/*; do | |
| if [[ -f "$file" ]]; then | |
| gh release upload "${{ github.ref_name }}" "$file" | |
| fi | |
| done | |
| done |
🤖 Prompt for AI Agents
In template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif
%}.jinja around lines 108 to 113, the shell loop uploading release assets does
not handle errors, which may cause silent failures. Modify the loop to check the
exit status of each gh release upload command and handle errors appropriately,
such as by logging an error message and exiting the script if an upload fails.
| path: ~/.cargo/bin/cross | ||
| key: cross-v0.2.1-${{ runner.os }} | ||
| - name: Install cross | ||
| run: cargo install cross --git https://github.com/cross-rs/cross --tag v0.2.1 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using a more flexible cross installation approach.
Hardcoding the cross version to v0.2.1 may lead to maintenance issues as newer versions become available. Consider using the latest stable version or making it configurable.
- name: Install cross
- run: cargo install cross --git https://github.com/cross-rs/cross --tag v0.2.1
+ name: Install cross
+ run: cargo install cross --git https://github.com/cross-rs/cross📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: cargo install cross --git https://github.com/cross-rs/cross --tag v0.2.1 | |
| name: Install cross | |
| run: cargo install cross --git https://github.com/cross-rs/cross |
🤖 Prompt for AI Agents
In template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif
%}.jinja at line 71, the cargo install command hardcodes the cross version to
v0.2.1. To fix this, modify the installation step to either install the latest
stable version by omitting the --tag option or make the version configurable via
a variable or input parameter, allowing easier updates and maintenance.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@sourcery-ai review |
leynos
deleted the
codex/add-selectable-repo-flavours-and-github-workflow
branch
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/test_template.py (1)
31-58: Excellent test separation addressing previous feedback.The separate test functions for each flavour significantly improve clarity and maintainability, exactly as suggested in the previous review. Each test independently verifies the correct source file generation and successful build for its respective flavour.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)copier.yaml(1 hunks)template/.github/workflows/{% if flavour == APP %}release.yml{% endif %}.jinja(1 hunks)template/src/{% if flavour == APP %}main.rs{% else %}lib.rs{% endif %}.jinja(1 hunks)tests/test_template.py(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
copier.yaml
[error] 37-37: too many blank lines
(1 > 0) (empty-lines)
🪛 Ruff (0.11.9)
tests/test_template.py
37-37: datetime.datetime.now() called without a tz argument
(DTZ005)
52-52: datetime.datetime.now() called without a tz argument
(DTZ005)
🔇 Additional comments (8)
README.md (1)
18-23: Documentation accurately reflects the new flavour functionality.The updated descriptions clearly explain the conditional behaviour for both starter code generation and release workflow inclusion based on the selected flavour. The documentation maintains consistency with the existing style.
template/src/{% if flavour == APP %}main.rs{% else %}lib.rs{% endif %}.jinja (1)
1-11: Well-implemented conditional source generation.The template correctly generates appropriate starter code for each flavour:
- APP flavour: Binary entry point with
main()function- LIB flavour: Library function with proper
#[must_use]attributeThe conditional logic is sound and the generated Rust code follows best practices.
copier.yaml (1)
25-36: Well-structured flavour configuration.The flavour option is properly implemented using YAML anchors and aliases for maintainability. The configuration includes appropriate choices, defaults, and help text.
tests/test_template.py (1)
10-11: Good use of constants for flavour values.The constants improve code readability and maintainability throughout the test file.
template/.github/workflows/{% if flavour == APP %}release.yml{% endif %}.jinja (4)
15-57: Comprehensive platform coverage in build matrix.The matrix strategy covers all major platforms and architectures, providing excellent cross-platform support for Rust applications. The configuration properly handles platform-specific details like file extensions.
65-83: Well-implemented build process with proper caching.The build steps demonstrate good practices:
- Appropriate caching of cross binary and cargo dependencies
- Use of cross-compilation tool for reliable cross-platform builds
- Proper Rust toolchain setup with minimal profile
84-95: Excellent artifact preparation with security considerations.The artifact preparation includes SHA256 checksum generation, which is a security best practice for release binaries. The structured naming convention makes artifacts easily identifiable.
97-115: Secure and well-structured release process.The release job properly:
- Depends on the build job completion
- Uses established GitHub Actions for release creation
- Handles artifact downloads and uploads securely
- Uses appropriate GitHub token authentication
| choices: *flavour_choices | ||
| default: *lib | ||
| help: 'Project flavour: generate a library crate or an application' | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove extra blank line to address linting warning.
Static analysis flagged an unnecessary blank line at the end of the file.
-
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 37-37: too many blank lines
(1 > 0) (empty-lines)
🤖 Prompt for AI Agents
In copier.yaml at line 37, remove the extra blank line at the end of the file to
resolve the linting warning about unnecessary trailing whitespace.
| tmp_path, | ||
| project_name="AppExample", | ||
| package_name="app_example", | ||
| license_year=datetime.now().year, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding timezone argument to datetime.now().
Static analysis suggests specifying a timezone argument for datetime.now() calls, though this is not critical for testing purposes.
- license_year=datetime.now().year,
+ license_year=datetime.now(tz=None).year,Also applies to: 52-52
🧰 Tools
🪛 Ruff (0.11.9)
37-37: datetime.datetime.now() called without a tz argument
(DTZ005)
🤖 Prompt for AI Agents
In tests/test_template.py at lines 37 and 52, the calls to datetime.now() lack a
timezone argument, which can lead to ambiguous datetime values. Update these
calls to include a timezone, such as datetime.now(timezone.utc), by importing
the timezone module from datetime. This ensures the datetime objects are
timezone-aware and consistent.
1 participant
Summary
flavourquestion incopier.yamlsrc/lib.rsorsrc/main.rsbased on flavourappflavourTesting
pip install -r requirements.txtpytest -qnpx -y markdownlint-cli2 '**/*.md'(fails: many lint warnings)cargo fmt,cargo clippy -- -D warnings, andcargo testrun on generated projecthttps://chatgpt.com/codex/tasks/task_e_6858453897b083228c3b798c3af142da
Summary by Sourcery
Introduce a "flavour" option in the copier template to generate either a library or application crate, update related templates, tests, and documentation, and add a GitHub Actions release workflow for binaries.
New Features:
Enhancements:
CI:
Documentation:
Tests:
Summary by CodeRabbit