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, Cleanup Code and use Generics #169

Merged
merged 8 commits into from
Jun 3, 2024
Merged

Conversation

jabdoa2
Copy link
Contributor

@jabdoa2 jabdoa2 commented May 9, 2024

  • Refactored handler to use generics instead of "podController" wrapper type
  • Deduplicated Getter Types
  • Deduplicated Controller logic and entrypoints
  • Deduplicated Webhook entrypoints
  • Deduplicated Controller Tests (using generics)
    • Tests are now exactly the same for all three objects (there were slightly diverged before)
    • Deduplicated podTemplate in Test Data

Overall we lost about 1.2k lines of code. Also extending/maintaining controllers and controller tests became easier. Before you basically had to make the same change in three files each. That is now in one place. Only downside is that you have to deal with generic types in tests now.

@jabdoa2 jabdoa2 requested a review from a team as a code owner May 9, 2024 23:20
@jabdoa2
Copy link
Contributor Author

jabdoa2 commented May 13, 2024

I will resolve the conflict. I also found why my tests for #167 failed. I will improve the test setup to make those easier to add.

pkg/controller/deployment/deployment_controller_test.go Outdated Show resolved Hide resolved
pkg/core/controller.go Outdated Show resolved Hide resolved
pkg/core/controller.go Outdated Show resolved Hide resolved
jabdoa2 and others added 2 commits June 3, 2024 16:08
Co-authored-by: Philipp Riederer <pt@philipptoelke.de>
Co-authored-by: Philipp Riederer <pt@philipptoelke.de>
@toelke toelke merged commit e748c7e into wave-k8s:master Jun 3, 2024
6 checks passed
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.

2 participants