Skip to content

fix(commit-conventions): address Copilot second-sweep finding from #27#28

Merged
CybotTM merged 1 commit into
mainfrom
fix/copilot-review-sweep-2
Apr 21, 2026
Merged

fix(commit-conventions): address Copilot second-sweep finding from #27#28
CybotTM merged 1 commit into
mainfrom
fix/copilot-review-sweep-2

Conversation

@CybotTM

@CybotTM CybotTM commented Apr 21, 2026

Copy link
Copy Markdown
Member

Follow-up to a Copilot comment on #27 (the follow-up to #26).

Changes

commit-conventions.md: rewrite the -S rationale. The prior phrasing said signing could silently fail when commit.gpgsign=true is set — that's inaccurate: git aborts noisily in that case. The real failure mode is that global config isn't loaded (subprocess env isolation, different $HOME, env scrubbing), so git doesn't realize signing was requested and creates an unsigned commit with no error. Explicit -S pins the requirement to the invocation so the 'config not loaded' case becomes visible.

Thread ID resolved after merge

  • PRRT_kwDOQoBsD858lFR9

The rationale for explicit '-S' was technically inaccurate: when
commit.gpgsign=true IS loaded, git aborts noisily on signing failure,
it doesn't silently fall back to unsigned. The real failure mode is
that global config isn't loaded — subprocess env isolation, different
$HOME, env scrubbing — so git doesn't know signing was requested in
the first place and creates an unsigned commit with no error. Explicit
-S pins the requirement to the invocation so the 'not loaded' case
becomes visible.

Thread: PRRT_kwDOQoBsD858lFR9
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Copilot AI review requested due to automatic review settings April 21, 2026 17:49
@CybotTM CybotTM merged commit b17fe3d into main Apr 21, 2026
11 checks passed
@CybotTM CybotTM deleted the fix/copilot-review-sweep-2 branch April 21, 2026 17:49
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the documentation regarding the use of the -S flag in Git commits to better explain how subprocess environments can miss global configurations. A review comment suggests refining the phrasing to more clearly distinguish between missing configurations and unreachable signing agents, while also recommending more precise terminology like 'creating' instead of 'shipping' for local commits.

Comment thread skills/git-workflow/references/commit-conventions.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines the documentation rationale for requiring explicit commit signing (-S) by correcting an inaccurate failure mode description and replacing it with the real-world case where global Git config isn’t loaded in certain subprocess environments.

Changes:

  • Rewrites the “Why explicit -S” explanation to focus on missing global config loading (e.g., different/scrubbed $HOME) rather than “silent failure” when signing is requested via config.
  • Clarifies that explicit -S forces signing to be requested at invocation time so failures become immediate and visible.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CybotTM added a commit that referenced this pull request Apr 21, 2026
fix(commit-conventions): refine -S rationale per #28 review
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.

2 participants