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 the helm and OCI chart clients and respective fakes. #3370

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

absoludity
Copy link
Contributor

@absoludity absoludity commented Sep 8, 2021

Description of the change

This is purely renaming and refactoring that I found myself wanting to do while trying to write tests for the CreateInstalledPackage endpoint of the helm plugin for the kubeapps apis server.

The renaming is from Resolver -> ChartClient , and the refactoring is to move the fakes for chart client and the chart client factory into the one file, rather than being spread out in different places.

Benefits

Clearer what the structs and interfaces are for. Enables me to continue writing tests happily :)

Possible drawbacks

None that I can see.

Applicable issues

Additional information

Signed-off-by: Michael Nelson <minelson@vmware.com>
Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, nice refactor, much clearer now what is each function/struct for!

Out of the scope of this PR, but wanted to say anyway :P What annoys me the most is still having the helm3to2 and the converters...
I wish we had time to clean up the /pkg folder someday

Well, indeed I started when I upgraded some go deps, but have to wait until helm 3.7 is released (this week perhaps?) because of some conflicts with the ORAS pkgs for managing OCI

@absoludity absoludity merged commit 21915d5 into master Sep 8, 2021
@absoludity absoludity deleted the fakeresolver-renamed branch September 8, 2021 11:28
@absoludity
Copy link
Contributor Author

Awesome, nice refactor, much clearer now what is each function/struct for!

Out of the scope of this PR, but wanted to say anyway :P What annoys me the most is still having the helm3to2 and the converters...

Yeah, there's quite a few pieces of tech-debt like that which will only get removed if we happen to touch the related code and can clean it up as a fly-by (like I've done here for this stuff). Or if it annoys us so much we can't stand it any longer...

I wish we had time to clean up the /pkg folder someday

Well, indeed I started when I upgraded some go deps, but have to wait until helm 3.7 is released (this week perhaps?) because of some conflicts with the ORAS pkgs for managing OCI

Sounds like it annoyed you too much :P, great if we can remove it once 3.7 is out.

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