Skip to content

Conversation

praveenrewar
Copy link
Member

@praveenrewar praveenrewar commented Mar 3, 2023

What this PR does / why we need it:

Expose values in kapp-controller package. Release on fork.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?

- Expose more values in kc package.
- Breaking change: Use lowerCamelCase for existing values.

Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:


@praveenrewar praveenrewar force-pushed the update-package-values branch from 651fe5e to e56281d Compare March 3, 2023 17:08
@praveenrewar praveenrewar requested a review from neil-hickey March 3, 2023 17:23
@praveenrewar praveenrewar force-pushed the update-package-values branch 4 times, most recently from 8a5b8b8 to 0e2377f Compare March 6, 2023 20:18
@praveenrewar praveenrewar marked this pull request as ready for review March 7, 2023 05:07
Copy link

@neil-hickey neil-hickey left a comment

Choose a reason for hiding this comment

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

Nice nice, thanks for this @praveenrewar !

For future us... we opted for renaming the existing values to use the lowerCamelCase so as not to have two different styles of keys. Given on March 8th, we had one download of the kc bundle over the last month. We agreed that this was better to break now while noone is using it :D

@praveenrewar praveenrewar force-pushed the update-package-values branch from 6f6653e to beb89f1 Compare March 9, 2023 14:48
@praveenrewar
Copy link
Member Author

While testing the kc package, I noticed that I had used

- #@ "-metricsBindAddress={}".format(data.values.metricsBindAddress)

instead of

- #@ "-metrics-bind-address={}".format(data.values.metricsBindAddress)

I have updated it now.
@neil-hickey Do you want to take a look at it again or I can go ahead and merge it?

@rohitagg2020
Copy link
Contributor

LGTM

@rohitagg2020 rohitagg2020 merged commit 53f7754 into carvel-dev:develop Mar 10, 2023
@praveenrewar praveenrewar deleted the update-package-values branch April 9, 2024 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants