harshilv17 commented on PR #6834: URL: https://github.com/apache/camel-k/pull/6834#issuecomment-5751505905
Re-reviewed my own change and found three things I had stated too strongly. Fixed in `6067e54` and `198d3293`, and the description is corrected. **1. `IsExternal() || IsSynthetic()` → `IsExternal()` is a reduction, not an equivalence.** I wrote that the branches "collapse", which invites you to skip the check. They are equal on every input except one: a Kit labelled `kit.type: synthetic`. I had also claimed nothing ever assigned that label. That is true of `main`, but not of history, and the difference matters here. `pkg/trait/container.go` did write it, from `a5c2ce7` until `515c762` removed it — which puts the writer in **2.4.0 only**. So a cluster upgraded from 2.4.0 can still hold such a Kit. Traced what happens to one: it loses the deprecation condition and the skip, `monitor.go` computes a digest, finds none stored, flips the phase to `Initialization`, and `initialize.go` returns it to `Ready` immediately on its `Spec.Image != ""` path. One status write and it settles — no loop, since the digest is written in the same pass that flips the phase. That seems right to me given `e51a99e`, where you removed the compat handling for the traits dropped in #6805 rather than keeping a shim, but it is your call and it should be stated rather than asserted away. Now under **Upgrade behaviour** in the description. **2. One of my renames was wrong.** `TestKameletSyntheticKitAutoConditionFalse` never touches the catalog — it only clears `Spec.Sources`, and its own comment says so. I had filed it under `...WithoutCatalog...`, which is exactly the mistake I said I was avoiding. It is `TestKameletNoSourcesAutoConditionFalse` now. **3. Smaller corrections.** There were 8 distinct identifiers, not 10 — 10 was the occurrence count including two call sites. `keytool` is in fact present here; only `mvn` is missing, and two of the `pkg/builder` failures are git/network rather than maven. The unrelated-failure list also omitted `pkg/kamelet/repository`. I verified the underlying claim properly this time: the failing-package sets on this branch and on a clean `origin/main` worktree are byte identical. Also restored a blank line in `integrationkit_types.go` that the constant removal had swallowed between the kit-type and layout-label groups. `go build ./...`, `go vet ./...`, `gofmt` and the `pkg/trait` and `pkg/apis/...` suites are clean at `198d3293`. Worth noting for whoever approves the runs: `pkg/controller/integrationkit` has no test files at all, so those three branches are covered by review only, before and after this change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
