Skip to content

Refactor: Commit message build, simplify Issue-ID handling#37

Merged
tykeal merged 1 commit intolfreleng-actions:mainfrom
modeseven-lfreleng-actions:update-action
Nov 5, 2025
Merged

Refactor: Commit message build, simplify Issue-ID handling#37
tykeal merged 1 commit intolfreleng-actions:mainfrom
modeseven-lfreleng-actions:update-action

Conversation

@ModeSevenIndustrialSolutions
Copy link
Copy Markdown
Contributor

Major Changes:

  1. Centralized Commit Message Building

    • Created _build_commit_message_with_trailers() as single source of truth
    • Consolidated duplicate trailer logic from 3 separate code paths
    • Now handles Issue-ID, Signed-off-by, Change-ID, and GitHub metadata
    • Ensures consistent trailer ordering across all modes (squash, single, PR-as-commit)
    • Reduced code by ~200 lines, improved maintainability
  2. Simplified Issue-ID Lookup

    • Removed ISSUE_ID_LOOKUP input (auto-enabled when JSON provided)
    • Moved JSON lookup logic from GitHub Actions to Python CLI
    • Removed dependency on external json-key-value-lookup-action
    • Added JSON validation with clear warning messages
    • Simplified action.yaml by ~50 lines (removed 3 workflow steps)
  3. Enhanced User Experience

    • Added Issue-ID to configuration table output
    • Added console feedback: "✅ Added Issue-ID {value} to commit message"
    • Added warning output: "⚠️ Warning: Issue-ID JSON was not valid"
    • Issue-ID now added in ALL modes (squash, single commits, USE_PR_AS_COMMIT)
  4. Code Quality

    • Fixed all pre-commit linting errors
    • Added issue_id_lookup_json parameter to all test fixtures
    • All tests passing with 67.2% coverage
    • Updated README documentation

Copilot AI review requested due to automatic review settings November 4, 2025 16:53
@github-actions github-actions Bot added the refactor Refactoring of code label Nov 4, 2025
Copy link
Copy Markdown

Copilot AI left a comment

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 moves Issue-ID lookup logic from GitHub Actions workflow steps to Python code, simplifying the action and centralizing trailer management. The change eliminates the external json-key-value-lookup-action dependency and consolidates commit message trailer handling into a single function.

Key changes:

  • Migrated Issue-ID JSON lookup from bash steps to Python (_resolve_issue_id_from_json function)
  • Removed ISSUE_ID_LOOKUP input (boolean toggle) - now lookup happens automatically when ISSUE_ID_LOOKUP_JSON is provided
  • Created centralized _build_commit_message_with_trailers function to handle all trailer ordering and insertion
  • Updated tests to verify lookup functionality is no longer in action steps

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/github2gerrit/cli.py Added _resolve_issue_id_from_json function and --issue-id-lookup-json CLI parameter; integrated lookup into main flow
src/github2gerrit/core.py Removed legacy _insert_issue_id_into_commit_message; added centralized _build_commit_message_with_trailers for consistent trailer management
src/github2gerrit/models.py Added issue_id_lookup_json field to Inputs dataclass
action.yaml Removed ISSUE_ID_LOOKUP input and JSON lookup steps; added GITHUB_ACTOR environment variable; disabled UV cache
.github/workflows/github2gerrit.yaml Removed ISSUE_ID_LOOKUP input from workflow definitions
README.md Updated documentation to reflect automatic lookup behavior and removed migration note
tests/*.py Added issue_id_lookup_json="" to all test Inputs instantiations; updated action step tests

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

Comment thread src/github2gerrit/core.py Outdated
Comment thread src/github2gerrit/core.py Outdated
Comment thread src/github2gerrit/cli.py Outdated
Major Changes:

1. Centralized Commit Message Building
   - Created _build_commit_message_with_trailers() as single source of truth
   - Consolidated duplicate trailer logic from 3 separate code paths
   - Now handles Issue-ID, Signed-off-by, Change-ID, and GitHub metadata
   - Ensures consistent trailer ordering across all modes
     (squash, single, PR-as-commit)
   - Reduced code by ~200 lines, improved maintainability

2. Simplified Issue-ID Lookup
   - Removed ISSUE_ID_LOOKUP input (auto-enabled when JSON provided)
   - Moved JSON lookup logic from GitHub Actions to Python CLI
   - Removed dependency on external json-key-value-lookup-action
   - Added JSON validation with clear warning messages
   - Simplified action.yaml by ~50 lines (removed 3 workflow steps)

3. Enhanced User Experience
   - Added Issue-ID to configuration table output
   - Added console feedback: "✅ Added Issue-ID {value} to commit message"
   - Added warning output: "⚠️ Warning: Issue-ID JSON was not valid"
   - Issue-ID now added in ALL modes (squash, single commits, USE_PR_AS_COMMIT)

4. Code Quality
   - Fixed all pre-commit linting errors
   - Added issue_id_lookup_json parameter to all test fixtures
   - All tests passing with 67.2% coverage
   - Updated README documentation

Signed-off-by: Matthew Watkins <[email protected]>
@tykeal tykeal merged commit 260cddc into lfreleng-actions:main Nov 5, 2025
17 checks passed
@ModeSevenIndustrialSolutions ModeSevenIndustrialSolutions deleted the update-action branch November 11, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactoring of code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants