refactor: drop MetricsConfig from ToolchainStatus type#1264
refactor: drop MetricsConfig from ToolchainStatus type#1264xcoulon merged 2 commits intocodeready-toolchain:masterfrom
Conversation
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughUpdated two module dependency versions in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 34-37: The two replace directives pointing
github.com/codeready-toolchain/api -> github.com/xcoulon/api and
github.com/codeready-toolchain/toolchain-common ->
github.com/xcoulon/toolchain-common must be accompanied by a tracked rollback
plan: add a clear comment above each replace in go.mod that states the reason,
the maintainer/owner, the explicit target date to remove the replace (e.g.,
YYYY-MM-DD), and a link to a new GitHub issue or PR that tracks "Remove
personal-fork replace when upstream releases change X"; also create that GitHub
issue titled to reference the modules (github.com/xcoulon/api and
github.com/xcoulon/toolchain-common) and set the milestone/assignee so the
replace will be removed on or before the target date. Ensure the comment
references the exact replace lines and the temporary fork versions currently
used.
In `@test/metrics/metrics_test.go`:
- Around line 707-709: The test calls hostAwait.UpdateToolchainConfig(t) with no
modifiers, which is inconsistent with other usages that pass explicit options;
either remove this call if no config change is needed, or replace it with a call
that includes the intended modifiers (for example
testconfig.AutomaticApproval().Enabled(true/false) or other testconfig.*
options) so the intent is explicit; look for UpdateToolchainConfig and replace
or remove it accordingly, aligning with neighboring tests that use
testconfig.AutomaticApproval().Enabled(...) as the pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b4fa444-9e0a-475c-b460-ad4f6e427408
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modtest/metrics/metrics_test.go
| replace github.com/codeready-toolchain/api => github.com/xcoulon/api v0.0.0-20260302105036-669d151cfca4 | ||
|
|
||
| replace github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common v0.0.0-20260304134451-ea974aea63a4 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
github.com/codeready-toolchain/api recent releases tags ToolchainStatus MetricsConfig
💡 Result:
- GitHub Releases (codeready-toolchain/api): There are no published releases in the repository. [1]
- Git tags (codeready-toolchain/api): The repository’s Tags page also shows no tags. [2]
- API types you mentioned:
ToolchainStatusis defined in theapi/v1alpha1package, andHostConfigincludes aMetrics MetricsConfigfield (i.e.,MetricsConfigexists/used there). [3]
Sources:
[1] (github.com)
[2] (github.com)
[3] (pkg.go.dev)
Citations:
- 1: https://github.com/codeready-toolchain/api/releases
- 2: https://github.com/codeready-toolchain/api/tags
- 3: https://pkg.go.dev/github.com/codeready-toolchain/api/api/v1alpha1?utm_source=openai
🌐 Web query:
github.com/codeready-toolchain/toolchain-common recent releases tags 2026
💡 Result:
As of March 4, 2026, the GitHub repo codeready-toolchain/toolchain-common shows:
- Releases: None published (so no 2026 releases). [1]
- Tags: The repo’s Tags page does not list any tags (so no 2026 tags visible). [2]
Sources: [1], [2]
🏁 Script executed:
# Check the current go.mod content and context around lines 34-37
cat go.mod | head -40Repository: codeready-toolchain/toolchain-e2e
Length of output: 1667
🏁 Script executed:
# Search for ToolchainStatus and MetricsConfig in the codebase to understand the change
rg "MetricsConfig|ToolchainStatus" --type go -A 2 -B 2 | head -50Repository: codeready-toolchain/toolchain-e2e
Length of output: 3215
Ensure personal-fork replace directives have documented rollback plans.
Lines 34 and 36 route two core modules to personal forks (github.com/xcoulon/*) with no published official versions available containing these changes. Official repos have no releases or tags. While these forks may be intentional for active development, leaving them on master creates maintainability and supply-chain risk if development stalls or the forks diverge.
Add a tracked rollback plan with an explicit target date to remove these directives once the required changes are published in the official repositories.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 34 - 37, The two replace directives pointing
github.com/codeready-toolchain/api -> github.com/xcoulon/api and
github.com/codeready-toolchain/toolchain-common ->
github.com/xcoulon/toolchain-common must be accompanied by a tracked rollback
plan: add a clear comment above each replace in go.mod that states the reason,
the maintainer/owner, the explicit target date to remove the replace (e.g.,
YYYY-MM-DD), and a link to a new GitHub issue or PR that tracks "Remove
personal-fork replace when upstream releases change X"; also create that GitHub
issue titled to reference the modules (github.com/xcoulon/api and
github.com/xcoulon/toolchain-common) and set the milestone/assignee so the
replace will be removed on or before the target date. Ensure the comment
references the exact replace lines and the temporary fork versions currently
used.
| t.Run("verify metrics are still correct after restarting pod", func(t *testing.T) { | ||
| // given | ||
| hostAwait.UpdateToolchainConfig(t, testconfig.Metrics().ForceSynchronization(true)) | ||
| hostAwait.UpdateToolchainConfig(t) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect UpdateToolchainConfig implementation and zero-arg handling.
echo "=== UpdateToolchainConfig implementation ==="
rg -n -C6 --type=go '\bfunc\s+\(.*\)\s+UpdateToolchainConfig\('
echo -e "\n=== No-arg calls (with just t) ==="
rg -n -C2 --type=go '\bUpdateToolchainConfig\(t\)'
echo -e "\n=== Calls with explicit modifiers ==="
rg -n -C2 --type=go '\bUpdateToolchainConfig\(t,\s*'Repository: codeready-toolchain/toolchain-e2e
Length of output: 9257
Remove UpdateToolchainConfig(t) or pass explicit modifiers.
This is the only call in the codebase without explicit modifiers. Every other call passes explicit options (e.g., testconfig.AutomaticApproval().Enabled(...)). Clarify the intent: either this subtest requires specific config changes or the call should be removed entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/metrics/metrics_test.go` around lines 707 - 709, The test calls
hostAwait.UpdateToolchainConfig(t) with no modifiers, which is inconsistent with
other usages that pass explicit options; either remove this call if no config
change is needed, or replace it with a call that includes the intended modifiers
(for example testconfig.AutomaticApproval().Enabled(true/false) or other
testconfig.* options) so the intent is explicit; look for UpdateToolchainConfig
and replace or remove it accordingly, aligning with neighboring tests that use
testconfig.AutomaticApproval().Enabled(...) as the pattern.
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MatousJobanek, rajivnathan, xcoulon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Signed-off-by: Xavier Coulon xcoulon@redhat.com
Summary by CodeRabbit
Chores
Tests