Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-1.12 CP] Issue #6728: support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers #7049

Merged
merged 5 commits into from Nov 17, 2023

Conversation

27149chen
Copy link
Contributor

@27149chen 27149chen commented Nov 2, 2023

cherry-pick #6917 to release 1.12: support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@github-actions github-actions bot added Area/Design Design Documents Dependencies Pull requests that update a dependency file Documentation has-changelog labels Nov 2, 2023
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Attention: 78 lines in your changes are missing coverage. Please review.

Comparison is base (3325a0c) 60.64% compared to head (5c44ed4) 60.61%.

Files Patch % Lines
...nternal/resourcemodifiers/strategic_merge_patch.go 36.61% 38 Missing and 7 partials ⚠️
internal/resourcemodifiers/resource_modifiers.go 75.47% 7 Missing and 6 partials ⚠️
internal/resourcemodifiers/json_patch.go 81.48% 7 Missing and 3 partials ⚠️
internal/resourcemodifiers/json_merge_patch.go 55.00% 6 Missing and 3 partials ⚠️
pkg/restore/restore.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.12    #7049      +/-   ##
================================================
- Coverage         60.64%   60.61%   -0.03%     
================================================
  Files               247      250       +3     
  Lines             26527    26683     +156     
================================================
+ Hits              16087    16175      +88     
- Misses             9314     9367      +53     
- Partials           1126     1141      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Lyndon-Li Lyndon-Li requested review from ywk253100 and removed request for reasonerjt November 6, 2023 06:23
@anshulahuja98
Copy link
Collaborator

@27149chen I am not sure if we should take a big change now to 1.12
I don't think this was in initial plan to pull to 1.12

support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers

Signed-off-by: lou <alex1988@outlook.com>
Signed-off-by: lou <alex1988@outlook.com>
@ywk253100
Copy link
Contributor

@27149chen You need to rebase the commit to resolve This branch is out-of-date with the base branch warning

@Lyndon-Li Lyndon-Li merged commit c042d47 into vmware-tanzu:release-1.12 Nov 17, 2023
23 of 24 checks passed
@anshulahuja98
Copy link
Collaborator

1 small nitpick - we missed making the PR title reflect the cherry picked feature.

@27149chen
Copy link
Contributor Author

@anshulahuja98 what's the correct title for cherry-pick prs?

@anshulahuja98
Copy link
Collaborator

Should have ideally included the original PR naming
something to indicate what this PR is for apart from the Issue number

@27149chen 27149chen changed the title Cherry-pick #6917 to release 1.12 [release-1.12 CP] Issue #6728: support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Design Design Documents Dependencies Pull requests that update a dependency file Documentation has-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants