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 resource-recommend controller #684

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Conversation

ozline
Copy link
Contributor

@ozline ozline commented Aug 27, 2024

(ฅ´ω`ฅ) This PR also links to the 2024GLCC Katalyst topic. According to the requirements of the organizing committee, the PR needs to be merged before October 27th. This PR has been submitted for two months. I hope the review can be accelerated.

2024GLCC task related PR summary:

Type Link
Issue #593
PR-Code #684
PR-Charts kubewharf/charts#17
PR-QuickStart kubewharf/katalyst-website#12 kubewharf/katalyst-website#11

What type of PR is this?

Enhancements

What this PR does / why we need it:

Refactor resourcerecommend controller, use Katalyst controller framework

Which issue(s) this PR fixes:

#593

Special notes for your reviewer:

Copy link

codecov bot commented Aug 27, 2024

Codecov Report

Attention: Patch coverage is 73.80952% with 11 lines in your changes missing coverage. Please review.

Project coverage is 56.03%. Comparing base (65905ce) to head (3b8ea47).
Report is 104 commits behind head on main.

Files with missing lines Patch % Lines
pkg/client/control/resourcerecommend.go 76.31% 6 Missing and 3 partials ⚠️
...lyst-controller/app/options/resourcerecommender.go 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #684      +/-   ##
==========================================
- Coverage   56.74%   56.03%   -0.71%     
==========================================
  Files         552      576      +24     
  Lines       52626    67022   +14396     
==========================================
+ Hits        29860    37557    +7697     
- Misses      19014    25467    +6453     
- Partials     3752     3998     +246     
Flag Coverage Δ
unittest 56.03% <73.80%> (-0.71%) ⬇️

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.

@ozline ozline changed the title [WIP] refactor resource-recommend controller refactor resource-recommend controller Sep 10, 2024
caohe
caohe previously approved these changes Oct 10, 2024

Verified

This commit was signed with the committer’s verified signature.
ozline ozlinex
@ozline
Copy link
Contributor Author

ozline commented Oct 26, 2024

@luomingmeng Hi, I have resolved the previous review comments, rechecked the module I was responsible for, and modified all files that did not meet the go import standard

I merged my previous commits into a new commit. Since it involved repository synchronization, I used git push --force-with-lease, which caused my previous commit records to be lost

@luomingmeng
Copy link
Collaborator

@luomingmeng Hi, I have resolved the previous review comments, rechecked the module I was responsible for, and modified all files that did not meet the go import standard

I merged my previous commits into a new commit. Since it involved repository synchronization, I used git push --force-with-lease, which caused my previous commit records to be lost

It doesn't matter; you can also squash the changes into several commits based on features or fixes. Just keep the number of commits reasonable.

@luomingmeng luomingmeng added the workflow/merge-ready merge-ready: code is ready and can be merged label Oct 27, 2024
@luomingmeng luomingmeng merged commit 4fe5c45 into kubewharf:main Oct 28, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow/merge-ready merge-ready: code is ready and can be merged
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

4 participants