-
Notifications
You must be signed in to change notification settings - Fork 1
Add a feat to auto sign off the commit if the environment is provided. #12
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
Conversation
WalkthroughThe changes introduce support for automatic commit message sign-off via a new command-line flag and environment variable, update documentation accordingly, and add methods to retrieve Git author information. The Tokio dependency is updated, and minor clarifications are made in template instructions. Unit tests are included for the new Git methods. Additionally, a GitHub Actions workflow step for artifact attestation is activated with adjusted parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Main
participant Git
participant Env
User->>CLI: Run command (optionally with --signoff)
CLI->>Main: Parse arguments
Main->>Env: Check GIT_AUTO_SIGNOFF variable
Main->>CLI: Check --signoff flag
alt Signoff required
Main->>Git: get_author_name()
Main->>Git: get_author_email()
Main->>Main: Append "Signed-off-by" to commit message
end
Main->>Main: Output commit message, proceed with commit
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 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 (
|
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: 2
🔭 Outside diff range comments (1)
src/git.rs (1)
26-26
: Fix Clippy linting issues for uninlined format args.Multiple trace! statements need to be updated to use modern Rust string formatting as flagged by the pipeline failures.
Apply these diffs to fix the Clippy linting issues:
- trace!("Opening repository at {}", path); + trace!("Opening repository at {path}");- trace!("The repository workdir is: {:?}", work_dir); + trace!("The repository workdir is: {work_dir:?}");- trace!("Failed to create commit: {}", e); + trace!("Failed to create commit: {e}");- trace!("Author email: {}", email); + trace!("Author email: {email}");- trace!("Failed to get author email: {}", e); + trace!("Failed to get author email: {e}");- trace!("Author name: {}", name); + trace!("Author name: {name}");- trace!("Failed to get author name: {}", e); + trace!("Failed to get author name: {e}");Also applies to: 32-32, 64-64, 77-77, 82-82, 95-95, 100-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.toml
(1 hunks)README.md
(2 hunks)src/cli.rs
(1 hunks)src/git.rs
(2 hunks)src/main.rs
(2 hunks)templates/system.txt
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Cargo Build & Test
src/git.rs
[error] 26-26: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("Opening repository at {}", path) to trace!("Opening repository at {path}").
[error] 32-32: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("The repository workdir is: {:?}", work_dir) to trace!("The repository workdir is: {work_dir:?}").
[error] 64-64: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("Failed to create commit: {}", e) to trace!("Failed to create commit: {e}").
[error] 77-77: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("Author email: {}", email) to trace!("Author email: {email}").
[error] 82-82: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("Failed to get author email: {}", e) to trace!("Failed to get author email: {e}").
[error] 95-95: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("Author name: {}", name) to trace!("Author name: {name}").
[error] 100-100: Clippy lint 'uninlined_format_args': variables can be used directly in the format!
string. Change trace!("Failed to get author name: {}", e) to trace!("Failed to get author name: {e}").
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Rust project - latest (stable)
- GitHub Check: Rust project - latest (nightly)
- GitHub Check: Rust project - latest (beta)
🔇 Additional comments (8)
Cargo.toml (1)
29-29
: Approve tokio version update: critical soundness fix in broadcast channelThe bump from
1.43.0
to1.44.2
addresses a critical soundness issue in the broadcast channel—previously, values that wereSend
but notSync
could be cloned without proper synchronization, risking data races. There are no breaking changes or new features in this patch, and it is strongly recommended to pull in this fix for thread safety.– No action needed beyond merging the update
– Cargo.toml line 29 already specifies the correcttokio = { version = "1.44.2", features = ["full"] }
src/git.rs (3)
70-86
: LGTM! Clean implementation of author email retrieval.The method correctly retrieves the user email from Git configuration with appropriate error handling and logging.
88-104
: LGTM! Clean implementation of author name retrieval.The method correctly retrieves the user name from Git configuration with appropriate error handling and logging.
200-224
: LGTM! Comprehensive unit tests for the new methods.The unit tests properly validate both methods return successful results with non-empty strings. The tests follow the existing pattern and handle missing test repository gracefully.
src/cli.rs (1)
50-56
: LGTM! Clean CLI argument implementation.The
--signoff
flag is properly implemented with clear help text and appropriate defaults. It follows the established patterns in the codebase.templates/system.txt (1)
15-15
: LGTM! Improved instruction clarity.The wording change makes the bullet point guidelines clearer by explicitly stating the five-point limit and emphasizing brevity.
README.md (1)
17-17
: LGTM! Good documentation of the new signoff feature.The documentation clearly explains the new auto sign-off functionality and environment variable configuration.
Also applies to: 50-50
src/main.rs (1)
111-111
: LGTM: Necessary change for sign-off functionality.Making the result mutable is required to append the sign-off information later in the code.
src/main.rs
Outdated
// Detect auto signoff from environment variable | ||
let need_signoff_from_env = env::var("GIT_AUTO_SIGNOFF") | ||
.map(|v| v == "1" || v.eq_ignore_ascii_case("true")) | ||
.unwrap_or(false); | ||
|
||
// If the --signoff option is enabled, add signoff to the commit message | ||
if cli.signoff || need_signoff_from_env { | ||
trace!("signoff option is enabled, will add signoff to the commit message"); | ||
let (author_name, author_email) = ( | ||
repository.get_author_name()?, | ||
repository.get_author_email()?, | ||
); | ||
|
||
// Add signoff to the commit message | ||
let signoff = format!("\n\nSigned-off-by: {} <{}>", author_name, author_email); | ||
result.push_str(&signoff); | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Well-implemented auto sign-off feature with proper error handling.
The implementation correctly handles both CLI flags and environment variables, with proper Git author information retrieval and standard sign-off formatting.
However, verify the environment variable naming consistency. The AI summary indicates a discrepancy between GIT_AUTO_SIGNOFF
used in the code and AIGITCOMMIT_SIGNOFF
mentioned in the README documentation.
Consider this minor refactor for better readability:
- // Detect auto signoff from environment variable
- let need_signoff_from_env = env::var("GIT_AUTO_SIGNOFF")
- .map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
- .unwrap_or(false);
-
- // If the --signoff option is enabled, add signoff to the commit message
- if cli.signoff || need_signoff_from_env {
+ // Check if signoff is needed from CLI flag or environment variable
+ let need_signoff_from_env = env::var("GIT_AUTO_SIGNOFF")
+ .map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
+ .unwrap_or(false);
+
+ if cli.signoff || need_signoff_from_env {
🏁 Script executed:
#!/bin/bash
# Description: Check for environment variable naming consistency in documentation
# Expected: Find references to both GIT_AUTO_SIGNOFF and AIGITCOMMIT_SIGNOFF to confirm discrepancy
echo "Searching for environment variable references..."
rg -i "GIT_AUTO_SIGNOFF|AIGITCOMMIT_SIGNOFF" --type md --type txt
Length of output: 399
Align environment variable naming for auto sign-off
The auto sign-off logic itself is solid—proper CLI flag handling, environment parsing, and Git author retrieval. However, there’s a mismatch between the code (using GIT_AUTO_SIGNOFF
) and the README (referencing AIGITCOMMIT_SIGNOFF
). Please unify these names:
- Update
README.md
to useGIT_AUTO_SIGNOFF
instead ofAIGITCOMMIT_SIGNOFF
,
or - Change the code to read
AIGITCOMMIT_SIGNOFF
if that prefix is preferred.
Once the naming is consistent, the implementation is good to go.
Optional readability refactor for the CLI/env check:
- // Detect auto signoff from environment variable
- let need_signoff_from_env = env::var("GIT_AUTO_SIGNOFF")
- .map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
- .unwrap_or(false);
-
- if cli.signoff || need_signoff_from_env {
+ // Check if signoff is requested via CLI flag or environment variable
+ let need_signoff = cli.signoff
+ || env::var("GIT_AUTO_SIGNOFF")
+ .map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
+ .unwrap_or(false);
+
+ if need_signoff {
📝 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.
// Detect auto signoff from environment variable | |
let need_signoff_from_env = env::var("GIT_AUTO_SIGNOFF") | |
.map(|v| v == "1" || v.eq_ignore_ascii_case("true")) | |
.unwrap_or(false); | |
// If the --signoff option is enabled, add signoff to the commit message | |
if cli.signoff || need_signoff_from_env { | |
trace!("signoff option is enabled, will add signoff to the commit message"); | |
let (author_name, author_email) = ( | |
repository.get_author_name()?, | |
repository.get_author_email()?, | |
); | |
// Add signoff to the commit message | |
let signoff = format!("\n\nSigned-off-by: {} <{}>", author_name, author_email); | |
result.push_str(&signoff); | |
} | |
// Check if signoff is requested via CLI flag or environment variable | |
let need_signoff = cli.signoff | |
|| env::var("GIT_AUTO_SIGNOFF") | |
.map(|v| v == "1" || v.eq_ignore_ascii_case("true")) | |
.unwrap_or(false); | |
if need_signoff { | |
trace!("signoff option is enabled, will add signoff to the commit message"); | |
let (author_name, author_email) = ( | |
repository.get_author_name()?, | |
repository.get_author_email()?, | |
); | |
// Add signoff to the commit message | |
let signoff = format!("\n\nSigned-off-by: {} <{}>", author_name, author_email); | |
result.push_str(&signoff); | |
} |
🤖 Prompt for AI Agents
In src/main.rs around lines 130 to 146, the environment variable name for auto
sign-off is set to GIT_AUTO_SIGNOFF, but the README references
AIGITCOMMIT_SIGNOFF. To fix this, choose one consistent environment variable
name and update either the code to read AIGITCOMMIT_SIGNOFF or the README to use
GIT_AUTO_SIGNOFF. Ensure the code and documentation match exactly to avoid
confusion.
- Change push-to-registry to false in `ghcr.yml` Signed-off-by: mingcheng <[email protected]>
- Expand Docker instructions with additional options and environment variables - Add note about write access for --commit option - Include --yes option tip Signed-off-by: mingcheng <[email protected]>
- Modify bullet point phrasing in system.txt - Clarify the use of optional bullet points Signed-off-by: mingcheng <[email protected]>
- Add signoff option to CLI arguments in `cli.rs` - Implement author name and email retrieval in `git.rs` - Append signoff to commit message in `main.rs` if enabled - Add tests for author name and email functions in `git.rs` Signed-off-by: mingcheng <[email protected]>
- Implement detection of GIT_AUTO_SIGNOFF environment variable - Enable signoff if environment variable is set to true or 1 - Update logic to combine CLI signoff option with environment variable check Signed-off-by: mingcheng <[email protected]>
- Add `AIGITCOMMIT_SIGNOFF` environment variable to README - Mention auto sign-off feature when variable is set to true Signed-off-by: mingcheng <[email protected]>
Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.44.1 to 1.44.2. - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](tokio-rs/tokio@tokio-1.44.1...tokio-1.44.2) --- updated-dependencies: - dependency-name: tokio dependency-version: 1.44.2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [openssl](https://github.com/sfackler/rust-openssl) from 0.10.71 to 0.10.72. - [Release notes](https://github.com/sfackler/rust-openssl/releases) - [Commits](sfackler/rust-openssl@openssl-v0.10.71...openssl-v0.10.72) --- updated-dependencies: - dependency-name: openssl dependency-version: 0.10.72 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
- Replace static string formatting with variable interpolation in log messages - Apply consistent formatting for trace logs across `git.rs` and `openai.rs` Signed-off-by: mingcheng <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Update `AIGITCOMMIT_SIGNOFF` to `GIT_AUTO_SIGNOFF` in README for consistency - Reflect the correct variable used in the code for auto signoff feature Signed-off-by: mingcheng <[email protected]>
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)
README.md (1)
17-17
: Out-of-date environment variable nameThe bullet still mentions
AIGITCOMMIT_SIGNOFF
; the implementation and later docs useGIT_AUTO_SIGNOFF
.-- Auto sign-off commit messages, if specified environment variable `AIGITCOMMIT_SIGNOFF` is set to `true` +- Auto sign-off commit messages when the environment variable `GIT_AUTO_SIGNOFF` is set to `true`
🧹 Nitpick comments (2)
README.md (2)
72-79
: IncludeGIT_AUTO_SIGNOFF
in the Docker example for completenessThe example demonstrates all other relevant variables; adding the new one avoids confusion.
-e OPENAI_API_PROXY='<the proxy address if you need>' \ + -e GIT_AUTO_SIGNOFF='true' \
82-97
: Minor grammar & clarity fixes-Tips: You can add `--yes` options to skip the confirm. +Tip: Add the `--yes` option to skip the confirmation prompt.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/ghcr.yml
(1 hunks)Cargo.toml
(1 hunks)README.md
(3 hunks)src/cli.rs
(1 hunks)src/git.rs
(5 hunks)src/main.rs
(7 hunks)src/openai.rs
(2 hunks)templates/system.txt
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- Cargo.toml
- templates/system.txt
- src/openai.rs
- src/cli.rs
- src/git.rs
- src/main.rs
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~96-~96: The word ‘confirm’ is not a noun. Did you mean “confirmation”?
Context: ...You can add --yes
options to skip the confirm. ### Git Hook The AIGitCommit
also...
(PREPOSITION_VERB)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Rust project - latest (beta)
- GitHub Check: Rust project - latest (nightly)
- GitHub Check: Rust project - latest (stable)
- GitHub Check: Rust project - latest (beta)
- GitHub Check: Rust project - latest (nightly)
- GitHub Check: Rust project - latest (stable)
🔇 Additional comments (1)
README.md (1)
49-50
: 👍 Environment section is now consistent
GIT_AUTO_SIGNOFF
is correctly documented here.
- name: Generate artifact attestation | ||
uses: actions/attest-build-provenance@v2 | ||
with: | ||
subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} | ||
subject-digest: ${{ steps.push.outputs.digest }} | ||
push-to-registry: false |
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.
Attestation step will fail on pull_request
events – add a guard
docker/build-push-action
does not populate steps.push.outputs.digest
when push: false
(which is the case for pull-requests).
Because the attestation step runs unconditionally, it receives an empty digest and the action errors out.
- name: Generate artifact attestation
+ # Only run when an image has actually been pushed
+ if: ${{ github.event_name != 'pull_request' }}
uses: actions/attest-build-provenance@v2
This keeps the workflow green for PRs while still producing provenance for main/tag pushes.
📝 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: Generate artifact attestation | |
uses: actions/attest-build-provenance@v2 | |
with: | |
subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} | |
subject-digest: ${{ steps.push.outputs.digest }} | |
push-to-registry: false | |
- name: Generate artifact attestation | |
# Only run when an image has actually been pushed | |
if: ${{ github.event_name != 'pull_request' }} | |
uses: actions/attest-build-provenance@v2 | |
with: | |
subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} | |
subject-digest: ${{ steps.push.outputs.digest }} | |
push-to-registry: false |
🤖 Prompt for AI Agents
In .github/workflows/ghcr.yml around lines 51 to 56, the attestation step runs
unconditionally and fails on pull_request events because
steps.push.outputs.digest is empty when push is false. Fix this by adding a
conditional guard to the attestation step so it only runs when the digest output
is available, typically by checking if the event is not a pull_request or if the
digest exists, preventing errors and keeping the workflow green for PRs.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores
Tests