Skip to content

Rename formatChartname to formatChartName#13310

Closed
myeunee wants to merge 1 commit intohelm:mainfrom
myeunee:update
Closed

Rename formatChartname to formatChartName#13310
myeunee wants to merge 1 commit intohelm:mainfrom
myeunee:update

Conversation

@myeunee
Copy link
Copy Markdown
Contributor

@myeunee myeunee commented Sep 8, 2024

Rename formatChartname to formatChartName for consistency

What this PR does / why we need it:
This PR renames the variable formatChartname to formatChartName to maintain naming consistency across the codebase. Consistent naming conventions improve code readability and maintainability.

Special notes for your reviewer:
This is a breaking change. Users will need to update their chart to reflect the new variable name format. No additional dependencies or other breaking changes are introduced by this PR.

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

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 8, 2024
Rename formatChartname to formatChartName for consistency

Signed-off-by: myeunee <myeunee@gmail.com>
@yxxhero
Copy link
Copy Markdown
Member

yxxhero commented Sep 8, 2024

@myeunee this is a beaking change. it requires users to update their chart.

@myeunee
Copy link
Copy Markdown
Contributor Author

myeunee commented Sep 9, 2024

@myeunee this is a beaking change. it requires users to update their chart.

Sorry. I updated the description. Is there anything else I can do?

Copy link
Copy Markdown
Member

@robertsirc robertsirc left a comment

Choose a reason for hiding this comment

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

This is a private function. LGTM.

@myeunee
Copy link
Copy Markdown
Contributor Author

myeunee commented Sep 9, 2024

Hi, I see that there are three workflows awaiting approval for this PR. If possible, I would appreciate it if the approvals could be reviewed soon. Thank you so much for your time and attention! ☺️

@robertsirc
Copy link
Copy Markdown
Member

Best way to get the conversation started with your PR is to attend the developer meeting Thursday at 9:30 AM PST

@yxxhero
Copy link
Copy Markdown
Member

yxxhero commented Sep 9, 2024

@myeunee this is a beaking change. it requires users to update their chart.

oh. sorry. I miss something.

}

func formatChartname(c *chart.Chart) string {
func formatChartName(c *chart.Chart) string {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You missed occurrences of this name change. Can you please get all of them.

@TerryHowe TerryHowe mentioned this pull request Sep 24, 2024
3 tasks
@gjenkins8
Copy link
Copy Markdown
Member

Thanks @myeunee for the contrib.

#13349 is the fixed version of this PR (Thanks @TerryHowe). Want to close in favor of this?

@mattfarina
Copy link
Copy Markdown
Collaborator

Note, the PR that was merged instead of this one preserved the commit and author from the work here. It fixed the issue on top of that work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants