Skip to content

feat: add warning deduplicator#1856

Open
TerryHowe wants to merge 2 commits intooras-project:mainfrom
TerryHowe:feature-warning-handler
Open

feat: add warning deduplicator#1856
TerryHowe wants to merge 2 commits intooras-project:mainfrom
TerryHowe:feature-warning-handler

Conversation

@TerryHowe
Copy link
Member

@TerryHowe TerryHowe commented Sep 12, 2025

What this PR does / why we need it:

Add a warning deduplicator and some tests for it. Gets a little bloat out of options/remote.go

@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.21%. Comparing base (338759d) to head (bda2340).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1856      +/-   ##
==========================================
+ Coverage   87.18%   87.21%   +0.03%     
==========================================
  Files         143      144       +1     
  Lines        5539     5532       -7     
==========================================
- Hits         4829     4825       -4     
+ Misses        423      421       -2     
+ Partials      287      286       -1     

☔ 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.

@TerryHowe TerryHowe force-pushed the feature-warning-handler branch from 848995a to 1ee4072 Compare September 12, 2025 17:03
@shizhMSFT shizhMSFT changed the title feature: add warning handler feat: add warning handler Sep 23, 2025
@shizhMSFT shizhMSFT requested a review from Copilot September 23, 2025 06:53
Copy link
Contributor

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 introduces a centralized warning handler to manage registry warnings and reduces code duplication. The change extracts warning handling logic from the Remote struct into a dedicated WarningHandler component with proper concurrency handling.

  • Adds a new WarningHandler type with thread-safe warning deduplication
  • Removes warning handling code from the Remote struct to reduce bloat
  • Provides comprehensive test coverage for the new warning handler functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
cmd/oras/internal/option/warning_handler.go New warning handler implementation with sync.Map for thread-safe deduplication
cmd/oras/internal/option/warning_handler_test.go Comprehensive test suite covering deduplication, concurrency, and multi-registry scenarios
cmd/oras/internal/option/remote.go Removes old warning handling code and integrates new WarningHandler
cmd/oras/internal/option/binary_target.go Removes manual warning map initialization that's no longer needed

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@TerryHowe TerryHowe force-pushed the feature-warning-handler branch from 1ee4072 to 3395408 Compare September 24, 2025 20:59
@Wwwsylvia Wwwsylvia requested a review from Copilot October 20, 2025 13:49
Copy link
Contributor

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +29 to +31
type warningHandler struct {
warned sync.Map
}
Copy link
Member

Choose a reason for hiding this comment

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

I found the name warningHandler and warned a bit hard to understand... I don't have a good suggestion though. Is it more like a warner pool or something?

Copy link
Member Author

@TerryHowe TerryHowe Nov 24, 2025

Choose a reason for hiding this comment

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

Sounds good. I renamed to warningDeduplicator and the warner is now seen

Copy link
Contributor

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


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

@TerryHowe TerryHowe changed the title feat: add warning handler feat: add warning deduplicator Nov 26, 2025
@TerryHowe
Copy link
Member Author

This is ready for review

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added stale Inactive issues or pull requests and removed stale Inactive issues or pull requests labels Feb 10, 2026
@TerryHowe TerryHowe added this to the v1.4.0 milestone Feb 15, 2026
@TerryHowe TerryHowe force-pushed the feature-warning-handler branch from 2b99cf9 to c476067 Compare February 15, 2026 15:05
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
@TerryHowe TerryHowe force-pushed the feature-warning-handler branch from c476067 to f2f80e5 Compare February 15, 2026 15:30
@TerryHowe TerryHowe requested a review from sabre1041 as a code owner March 18, 2026 13:40
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.

3 participants