Skip to content

Conversation

robert-cronin
Copy link
Contributor

@robert-cronin robert-cronin commented Jul 28, 2025

  • Breaks patch.go logic out into separate files in preparation for buildkit frontend and generate command
  • Adds common package with Options to standardize copacetic options

Closes #1227

@robert-cronin robert-cronin self-assigned this Jul 28, 2025
@robert-cronin robert-cronin changed the title Modularize patching logic feat: modularize patching logic Jul 28, 2025
Copy link

codecov bot commented Jul 28, 2025

Codecov Report

❌ Patch coverage is 36.64825% with 688 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.86%. Comparing base (cb40f84) to head (2e653b3).

Files with missing lines Patch % Lines
pkg/patch/single.go 39.41% 168 Missing and 18 partials ⚠️
pkg/patch/multi.go 37.34% 149 Missing and 7 partials ⚠️
pkg/patch/manifest.go 0.00% 99 Missing ⚠️
pkg/patch/core.go 0.00% 86 Missing ⚠️
pkg/patch/patch.go 29.41% 34 Missing and 2 partials ⚠️
pkg/patch/platform.go 64.89% 22 Missing and 11 partials ⚠️
pkg/patch/build.go 58.20% 25 Missing and 3 partials ⚠️
pkg/common/buildkit.go 10.34% 25 Missing and 1 partial ⚠️
pkg/patch/cmd.go 5.00% 19 Missing ⚠️
pkg/patch/loader.go 23.52% 13 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1228      +/-   ##
==========================================
+ Coverage   42.65%   42.86%   +0.21%     
==========================================
  Files          24       35      +11     
  Lines        3946     4122     +176     
==========================================
+ Hits         1683     1767      +84     
- Misses       2135     2224      +89     
- Partials      128      131       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@robert-cronin robert-cronin force-pushed the feat/modularize-patching-logic branch 3 times, most recently from b4acadb to 323400f Compare July 28, 2025 05:45
@robert-cronin robert-cronin moved this from 🆕 New to 🏗 In progress in Copacetic Workboard Jul 28, 2025
@robert-cronin robert-cronin force-pushed the feat/modularize-patching-logic branch 2 times, most recently from 144a58d to e6d155a Compare July 28, 2025 06:42
@robert-cronin robert-cronin force-pushed the feat/modularize-patching-logic branch from e6d155a to aa818d9 Compare July 28, 2025 06:47
@robert-cronin robert-cronin marked this pull request as ready for review July 28, 2025 07:06
@Copilot Copilot AI review requested due to automatic review settings July 28, 2025 07:06
Copy link
Contributor

@Copilot 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 modularizes the patching logic by breaking down the monolithic patch.go file into separate, focused modules and introduces a standardized Options struct for consistent parameter passing across the copacetic system.

Key changes:

  • Splits large patch.go into focused modules (single.go, multi.go, platform.go, etc.)
  • Introduces types.Options struct to standardize copacetic configuration
  • Creates common package with shared utilities for OS detection, progress display, and platform handling

Reviewed Changes

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

Show a summary per file
File Description
pkg/types/options.go Introduces standardized Options struct for copacetic configuration
pkg/patch/single.go Handles single-architecture image patching logic
pkg/patch/multi.go Manages multi-platform image patching workflow
pkg/patch/platform.go Contains platform-specific utilities and filtering logic
pkg/patch/core.go Provides core patching functionality for reuse by different frontends
pkg/patch/manifest.go Handles multi-platform manifest creation and management
pkg/patch/build.go Manages build configuration and source policy validation
pkg/patch/loader.go Contains image loader detection logic
pkg/patch/patch.go Refactored main patch orchestration with simplified interface
pkg/patch/cmd.go Updates command interface to use new Options struct
pkg/patch/patch_test.go Updates tests to work with new modular structure
pkg/common/*.go New shared utilities for OS detection, progress display, and platform handling
Comments suppressed due to low confidence (2)

pkg/patch/platform.go:121

  • The fmt.Sprintf call inside the log statement creates unnecessary string formatting overhead. Consider using platforms.Format(*it.PatchedDesc.Platform) or constructing the string directly in the log call.
		return nil, fmt.Errorf("error getting manifest: %w", err)

pkg/patch/cmd.go:21

  • [nitpick] The field name was changed from reportFile to report but this may be confusing since the field can represent both a file path or directory path. Consider reportPath for clarity.
	report        string

@robert-cronin robert-cronin force-pushed the feat/modularize-patching-logic branch from aa818d9 to c91b256 Compare July 28, 2025 07:15
Signed-off-by: robert-cronin <robert.owen.cronin@gmail.com>
@robert-cronin robert-cronin force-pushed the feat/modularize-patching-logic branch from c91b256 to 5e6d3f3 Compare July 28, 2025 07:26
Copy link
Contributor

@ashnamehrotra ashnamehrotra left a comment

Choose a reason for hiding this comment

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

LGTM

@robert-cronin robert-cronin merged commit e5d2748 into project-copacetic:main Aug 4, 2025
28 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Copacetic Workboard Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[REQ] Modularize Patching Logic
2 participants