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

refactor: replace runtime.Object with metav1.Object in pkg/util/finalizers #270

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

z1cheng
Copy link
Contributor

@z1cheng z1cheng commented Nov 17, 2023

This PR is to replace runtime.Object with metav1.Object in pkg/util/finalizers

related to #99

@CLAassistant
Copy link

CLAassistant commented Nov 17, 2023

CLA assistant check
All committers have signed the CLA.

@z1cheng z1cheng changed the title Replace runtime.Object with metav1.Object in pkg/util/finalizers refactor: replace runtime.Object with metav1.Object in pkg/util/finalizers Nov 17, 2023
@z1cheng z1cheng force-pushed the z1cheng/fix_obj_usage branch from 4a1df35 to 822ea79 Compare November 18, 2023 10:55
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

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

Comparison is base (dcf2c66) 28.87% compared to head (94ee52f) 28.89%.

Files Patch % Lines
pkg/controllers/sync/controller.go 0.00% 13 Missing ⚠️
pkg/controllers/sync/dispatch/unmanaged.go 0.00% 6 Missing ⚠️
pkg/controllers/federate/controller.go 0.00% 4 Missing ⚠️
pkg/controllers/sync/resource.go 0.00% 4 Missing ⚠️
pkg/controllers/federatedcluster/controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   28.87%   28.89%   +0.02%     
==========================================
  Files         114      114              
  Lines       13590    13545      -45     
==========================================
- Hits         3924     3914      -10     
+ Misses       9283     9252      -31     
+ Partials      383      379       -4     
Flag Coverage Δ
unittests 28.89% <31.70%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@gary-lgy
Copy link
Member

@z1cheng Could you amend the PR description to remove the close in close #99? This PR doesn't fully close it, since the annotation util functions have not been fully refactored yet.

@z1cheng z1cheng requested a review from gary-lgy November 21, 2023 08:42
@z1cheng
Copy link
Contributor Author

z1cheng commented Nov 21, 2023

Submit a commit to replace the deprecated sets.String with generic sets.Set

@gary-lgy
Copy link
Member

@z1cheng pls rebase the PR (and keep it rebased), address the comment and then LGTM. Don't worry about the codecov/patch check failure.

@z1cheng z1cheng force-pushed the z1cheng/fix_obj_usage branch from 18f4e4c to 26de07c Compare November 22, 2023 13:11
@z1cheng z1cheng requested a review from gary-lgy November 22, 2023 13:14
@z1cheng
Copy link
Contributor Author

z1cheng commented Nov 25, 2023

@gary-lgy @SOF3 This looks like it can be merged, please let me know what else is needed :)

@gary-lgy
Copy link
Member

@gary-lgy @SOF3 This looks like it can be merged, please let me know what else is needed :)

The branch is not rebased onto the latest master. (another PR was merged in the meantime).
image

Pls keep it rebased.

@z1cheng z1cheng force-pushed the z1cheng/fix_obj_usage branch from 26de07c to 94ee52f Compare November 27, 2023 04:11
@z1cheng z1cheng requested review from gary-lgy and SOF3 November 27, 2023 04:37
@gary-lgy gary-lgy merged commit 60ba3c8 into kubewharf:main Nov 28, 2023
6 of 7 checks passed
@z1cheng z1cheng deleted the z1cheng/fix_obj_usage branch November 28, 2023 03:38
miraclejzd pushed a commit to miraclejzd/kubeadmiral that referenced this pull request Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants