Skip to content

Add a .diff target for k8s_deploy #55

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
Feb 12, 2025
Merged

Conversation

borg286
Copy link
Contributor

@borg286 borg286 commented Feb 6, 2025

Add a .diff target for k8s_deploy

Description

Add to the list of generated targets for k8s_deploy so, in addition to .show and .delete, it also has .diff

Related Issue

Motivation and Context

As part of a CICD pipeline, or testing, I'd like to see and record what the diff is between source and what kubernetes is actually running.

How Has This Been Tested?

No. Can you point me to how to test this?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [ x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ x ] My code follows the code style of this project.
  • [ x ] My change requires a change to the documentation.
  • [ x ] I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@michaelschiff
Copy link
Collaborator

gave this a try, saw the expected diffs. given that this just exposes a kubectl subcommand without additional changes I’m inclined to say its okay without further testing. the change makes sense to me, and I can see myself using it - though I start to wonder at what point the growing number of targets yielded by the macro becomes more annoying than piping the build output to kubectl and calling whatever subcommand you want.

lgtm

@borg286
Copy link
Contributor Author

borg286 commented Feb 10, 2025

What steps are next to get this merged in. This is my first PR on GitHub. Apologies for my ignorance

@borg286
Copy link
Contributor Author

borg286 commented Feb 11, 2025

gave this a try, saw the expected diffs. given that this just exposes a kubectl subcommand without additional changes I’m inclined to say its okay without further testing. the change makes sense to me, and I can see myself using it - though I start to wonder at what point the growing number of targets yielded by the macro becomes more annoying than piping the build output to kubectl and calling whatever subcommand you want.

lgtm

What are the next steps here?

@apesternikov apesternikov self-requested a review February 12, 2025 00:47
Copy link
Contributor

@apesternikov apesternikov left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

@apesternikov apesternikov merged commit 9b7f116 into fasterci:main Feb 12, 2025
2 checks passed
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