Content-Length: 304246 | pFad | https://github.com/helm/helm/pull/13600

D8 cleanup: `NewShowWithConfig` -> `NewShow` by gjenkins8 · Pull Request #13600 · helm/helm · GitHub
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

cleanup: NewShowWithConfig -> NewShow #13600

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

gjenkins8
Copy link
Member

What this PR does / why we need it:
NewShowWithConfig replaced NewShow . Remove NewShow, and rename NewShowWithConfig to NewShow .

Special notes for your reviewer:

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

Signed-off-by: George Jenkins <gvjenkins@gmail.com>
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 5, 2025

// NewShowWithConfig creates a new Show object with the given configuration.
func NewShowWithConfig(output ShowOutputFormat, cfg *Configuration) *Show {
func NewShow2(output ShowOutputFormat, cfg *Configuration) *Show {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func NewShow2(output ShowOutputFormat, cfg *Configuration) *Show {
func NewShow(output ShowOutputFormat, cfg *Configuration) *Show {

Copy link
Collaborator

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

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

This PR keeps the existing NewShow and renames NewShowWIthConfit to NewShow2. I don't think this was your intent. Is this half done? Can we get the full NewShow -> NewShowWithConfig conversion?

@gjenkins8 gjenkins8 force-pushed the cleanup_NewShowWithConfig branch from 708e83e to 8468de4 Compare January 7, 2025 17:48
Copy link
Contributor

@TerryHowe TerryHowe left a comment

Choose a reason for hiding this comment

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

/lgtm

@gjenkins8
Copy link
Member Author

This PR keeps the existing NewShow and renames NewShowWIthConfit to NewShow2. I don't think this was your intent. Is this half done? Can we get the full NewShow -> NewShowWithConfig conversion?

I missed a commit sorry (During the removal I renamed NewConfig -> NewConfig2 to find callers of that)

@gjenkins8 gjenkins8 requested a review from mattfarina January 12, 2025 23:39
@mattfarina mattfarina added this to the v4 milestone Jan 14, 2025
@mattfarina
Copy link
Collaborator

@gjenkins8 please feel free to merge when you're ready. It's approved.

@gjenkins8 gjenkins8 merged commit 53a4a59 into helm:main Jan 14, 2025
5 checks passed
@gjenkins8 gjenkins8 deleted the cleanup_NewShowWithConfig branch January 14, 2025 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/helm/helm/pull/13600

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy