Address review feedback from PRs #14 and #16 - #23
Conversation
Addresses all inline review comments from perdasilva:
catalog.go:
Move FBC schema strings ("olm.package", "olm.bundle", "olm.channel")
to named constants (fbcSchemaPackage/Bundle/Channel).
collector.go:
- Add ClusterServiceVersion, Subscription, InstallPlan to olmv0OnlyKinds
so all OLMv0 management resources are excluded in one place
- Remove redundant local skipKinds in gatherResourcesFromOperatorCR; use
olmv0OnlyKinds directly
- Rename gatherResourcesByOwnerLabel param csvName → ownerName
- Rename gatherResourcesByOwnerRef to take ownerName string rather than
csv *ClusterServiceVersion; update ownerRef check and call site
- Add comment explaining why Namespace is in possibleResourceGVKs
- Clarify deprecated status.install fallback with TODO comment
labels.go / migration.go:
- Extract cosWaitPollInterval/Timeout, ceWaitPollInterval/Timeout, and
subWaitPollInterval/Timeout constants
- Add TODO for SecretPacker exportability/configurability
scan.go:
- Replace custom splitNamespacedName with strings.SplitN (renamed splitSubRef)
compatibility.go:
- Fix stale C3 comment — APIService is not supported in OLMv1 (permanent
hard block); restore checkNoAPIServices and call site
specs/20260821-migration-v0-to-v1/requirements.md:
- C3 updated to permanent hard block; remove OPRUN-4723 future-removal note
specs/20260821-migration-v0-to-v1/plan.md:
- Phase 3 and Phase 7 notes updated to reflect C3 as permanent
migrate-catalogs-v0-to-v1/main.go:
- Move result status strings to named constants
convert.go:
- Add TODO about progress channel to avoid step-by-step duplication
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Todd Short <tshort@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Addresses all inline review comments from @perdasilva on the merged stack PRs.
Changes
catalog.go"olm.package","olm.bundle","olm.channel") to named constantsfbcSchemaPackage,fbcSchemaBundle,fbcSchemaChannelcollector.goClusterServiceVersion,Subscription,InstallPlantoolmv0OnlyKindsso all OLMv0 management resources are excluded in one placeskipKindsmap ingatherResourcesFromOperatorCR; useolmv0OnlyKindsdirectlygatherResourcesByOwnerLabelparamcsvName→ownerName(addresses suggestion)gatherResourcesByOwnerRefto takeownerName stringinstead ofcsv *ClusterServiceVersion; update ownerRef check and call site (addresses suggestion)Namespaceis inpossibleResourceGVKs(some operators create namespaces as bundle resources)status.installfallback with TODO commentlabels.go/migration.gocosWaitPollInterval/Timeout,ceWaitPollInterval/Timeout,subWaitPollInterval/Timeoutconstants (addresses suggestion to move timeout/poll periods to constants)secretPackerexportability/configurabilityscan.gosplitNamespacedNamewithstrings.SplitN(renamedsplitSubRefto be more specific)compatibility.gocheckNoAPIServicesand its call sitespecs/20260821-migration-v0-to-v1/requirements.md+plan.mdmigrate-catalogs-v0-to-v1/main.gostatusCreated,statusAdopted,statusSkipped,statusError,statusDryRun)convert.goMigrate()logicReviewer Checklist