Skip to content

feat: Capability to diff changes in chart hooks #72

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

Merged
merged 1 commit into from
Jul 25, 2018

Conversation

rmartinez3
Copy link
Contributor

Currently helm diff does not parse helm hooks. This appends hooks to the manifest diffs so that it can include them to be diff. Added capability to revision,upgrade and rollback.

@mumoshu mumoshu changed the title adding capability of parsing hooks for diffs feat: Capability to diff changes in chart hooks Jul 25, 2018
Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for the great work @rmartinez3.

The longer explanation why I'm accepting this follows.

I thought it would be better to completely differentiate diffs for hooks and manifests, but eventually switched myself to believe this is much better as a starting point because it requires minimal change to the code-base.

@mumoshu mumoshu merged commit fa12c4c into databus23:master Jul 25, 2018
@MattKetmo
Copy link
Contributor

MattKetmo commented Jan 23, 2019

Hello, can it make sense to have an option to disable hooks diff (like the --no-hooks for helm upgrade). I see cases where it can be annoying, mainly when hooks contains randomly generated values. For example the test hook in stable/jenkins uses a pod with a random name) (which is legitimate since it's just for a test), but then helm diff will always show me a diff (which I'd like to avoid because there is nothing to upgrade).
I can open a separate issue if you think it makes sense.
Thanks

@mumoshu
Copy link
Collaborator

mumoshu commented Jan 23, 2019

@MattKetmo Hey! That's a good point. Would you mind submitting a PR for that?

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.

3 participants