🌱 Use the module version of json-patch v5 #321
Conversation
WalkthroughThe changes update the import path and version of the Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
test/integration/cloudevents/agent_deploy_test.go (1)
29-29: Package aliasworkcollides with the local variableworkused throughout the testsInside multiple test cases a variable named
workis declared:var work *workv1.ManifestWorkThis silently shadows the imported package alias
workin those scopes, which is legal but error-prone and confusing when reading or refactoring the code.Consider one of:
-import "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/work" +import workclient "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/work"or renaming the local variable to
mw/manifestWork.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
go.mod(2 hunks)pkg/utils/helpers.go(1 hunks)test/integration/cloudevents/agent_deploy_test.go(2 hunks)test/integration/cloudevents/agent_hook_deploy_test.go(2 hunks)test/integration/cloudevents/agent_hosting_deploy_test.go(2 hunks)test/integration/cloudevents/agent_hosting_hook_deploy_test.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: cloudevents-integration
- GitHub Check: integration
- GitHub Check: e2e-hosted
- GitHub Check: build
- GitHub Check: e2e
- GitHub Check: unit
- GitHub Check: e2e-cloudevents
- GitHub Check: verify
🔇 Additional comments (6)
go.mod (1)
6-6: Dependency path correctly updated to the v5 moduleSwitching the direct requirement to
github.com/evanphx/json-patch/v5 v5.9.11is the right move and unblocks future drops of the legacy import path.pkg/utils/helpers.go (1)
12-12: Import path update looks goodThe alias is unchanged, so no downstream edits are needed. 👍
test/integration/cloudevents/agent_hosting_hook_deploy_test.go (1)
9-9: Test now tracks the v5 moduleImport path change is correct; nothing else to do.
test/integration/cloudevents/agent_hook_deploy_test.go (1)
9-9: Import path updatedLooks good – tests will compile against the new module.
test/integration/cloudevents/agent_hosting_deploy_test.go (1)
9-9: v5 import path appliedNo issues spotted with the switch here.
test/integration/cloudevents/agent_deploy_test.go (1)
21-23: LGTM – internal imports merely re-orderedOnly the import block was re-arranged; no functional impact.
| "open-cluster-management.io/addon-framework/pkg/agent" | ||
| "open-cluster-management.io/addon-framework/pkg/utils" | ||
|
|
||
| jsonpatch "github.com/evanphx/json-patch/v5" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure go.mod / go.sum are tidied after the major-version bump
The import path now points to the v5 module. Double-check that:
-
go.modcontainsgithub.com/evanphx/json-patch/v5 v5.9.11 // indirect?and that the old
github.com/evanphx/json-patchline is gone. -
go mod tidyhas been run so no redundant entries remain.
🏁 Script executed:
#!/bin/bash
# Fails if any pre-v5 import or module reference is still present.
rg --no-heading -n $'github.com/evanphx/json-patch"' || true
grep -n 'github.com/evanphx/json-patch"' go.{mod,sum} || trueLength of output: 124
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "🔍 Checking for v5 module require in go.mod..."
grep -nE 'github.com/evanphx/json-patch/v5 ' go.mod || echo "⚠️ v5 module not found in go.mod"
echo "🔍 Checking for v5 references in go.sum..."
grep -nE 'github.com/evanphx/json-patch/v5 ' go.sum || echo "⚠️ v5 references not found in go.sum"
echo "🔍 Ensuring no pre-v5 references remain in go.mod or go.sum..."
# Match the base path without `/v5`
grep -nE 'github\.com/evanphx/json-patch($| )' go.mod go.sum && {
echo "❌ Found pre-v5 references"
exit 1
} || echo "✅ No pre-v5 references detected"
echo "✅ Verification complete."Length of output: 1124
Remove lingering pre-v5 json-patch entries in go.mod/go.sum
The verification script found that the old, incompatible v5 entry is still present:
- go.mod:55
github.com/evanphx/json-patch v5.9.0+incompatible // indirect - go.sum:77–78
github.com/evanphx/json-patch v5.9.0+incompatible h1:… github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:…
Please remove those lines and ensure only the /v5 module remains:
Suggested diff:
--- go.mod
@@
- github.com/evanphx/json-patch v5.9.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.9.11 // indirect
--- go.sum
@@
- github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls=
- github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjTM0wiaDU=
github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM=After updating, run:
go get github.com/evanphx/json-patch/[email protected]
go mod tidy
to purge any residual entries and keep your module files clean.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In test/integration/cloudevents/agent_deploy_test.go around line 9, remove the
old incompatible v5 entry for github.com/evanphx/json-patch from go.mod and
go.sum files. Then run 'go get github.com/evanphx/json-patch/[email protected]'
followed by 'go mod tidy' to update dependencies and clean up any lingering
references, ensuring only the proper /v5 module remains.
|
/cc @qiujian16 |
|
/retest |
|
@skitt: Cannot trigger testing until a trusted user reviews the PR and leaves an In 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 kubernetes-sigs/prow repository. |
This matches other dependencies and means the non-modular will be dropped once other dependencies are bumped to versions that no longer pull it in. Signed-off-by: Stephen Kitt <[email protected]>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, skitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 5d62be0
into
open-cluster-management-io:main
3 participants
Summary
This matches other dependencies and means the non-modular will be dropped once other dependencies are bumped to versions that no longer pull it in.
Related issue(s)
Fixes #
Summary by CodeRabbit
json-patchlibrary to improve compatibility.