oras
oras copied to clipboard
refactor: simplify manifest index metadata handler
What this PR does / why we need it:
This is kind of an example of why composition can be better than inheritance.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 84.69%. Comparing base (
a38c2df) to head (6e8250b). Report is 43 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1673 +/- ##
==========================================
- Coverage 84.70% 84.69% -0.02%
==========================================
Files 126 126
Lines 5682 5678 -4
==========================================
- Hits 4813 4809 -4
Misses 619 619
Partials 250 250
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Hi @TerryHowe, thanks for your contribution.
Having separate OnIndexCreated and Render looks unnecessary, but this is our design to ensure future extensibility. Let me explain.
First of all, we need metadata.Render method for almost all commands' metadata handler, to signal that no more operations are coming and metadata can be printed. It has to be implemented by discard, and as go does not support method overloading, the Render of every command has to have the same parameters. As a result, we made it an interface and almost all metadata handlers implement it.
But we do need to pass in data for some commands, that's why we have functions like OnIndexCreated and OnAttached. In this way, when we add a command in the future, we can implement the metadata handler and discard neatly.
So we are not going to proceed with this PR, but thanks for your work anyway.
I think there are a couple more that do not need the Render method. There is no need to rigidly follow the same pattern for all metadata handlers when it adds no value. It adds complexity and confusion.
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.