Revert "Revert "NO-JIRA: Disable WatchList feature gate due to the missing support of Project watch"" - #751
Revert "Revert "NO-JIRA: Disable WatchList feature gate due to the missing support of Project watch""#751xueqzhan wants to merge 1 commit into
Conversation
…ssing support of Project watch""
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@xueqzhan: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview |
|
@xueqzhan: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/45811cc0-95d8-11f1-8d48-8fc9baf84d76-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview |
|
@xueqzhan: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/47cf0460-95d8-11f1-81e5-bb3713bcedaf-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview |
|
@xueqzhan: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/49ef4c50-95d8-11f1-9a5d-d6cfcee4acbb-0 |
|
[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 |
WalkthroughThe configuration observer now wraps its feature-gate accessor. The wrapper forces ChangesFeature-gate override
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/operator/configobservation/configobservercontroller/observe_config_controller.go (1)
91-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for the feature-gate wrapper.
Test error propagation,
WatchList=false, delegation of other gates, and single insertion ofWatchListintoKnownFeatures. Include a case where the underlying gate does not listWatchList, and verify the observer emitsWatchList=false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/operator/configobservation/configobservercontroller/observe_config_controller.go` around lines 91 - 129, Add focused tests for newFeatureGateAccessWithWatchListDisabled and featureGateWithWatchListDisabled covering CurrentFeatureGates error propagation, forced WatchList=false, delegation of non-WatchList gates, and KnownFeatures retaining exactly one WatchList entry whether or not the underlying gate lists it. Include an observer-level case with WatchList absent from the underlying gate and verify the emitted feature gate value is false.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@pkg/operator/configobservation/configobservercontroller/observe_config_controller.go`:
- Around line 91-129: Add focused tests for
newFeatureGateAccessWithWatchListDisabled and featureGateWithWatchListDisabled
covering CurrentFeatureGates error propagation, forced WatchList=false,
delegation of non-WatchList gates, and KnownFeatures retaining exactly one
WatchList entry whether or not the underlying gate lists it. Include an
observer-level case with WatchList absent from the underlying gate and verify
the emitted feature gate value is false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bd67ee78-7792-4af9-b7c5-6b0c1377859b
📒 Files selected for processing (1)
pkg/operator/configobservation/configobservercontroller/observe_config_controller.go
Reverts #681
/hold
testing for now. Please ignore.
Summary by CodeRabbit