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

feat: add --ref-path to render to different path than target-branch #228

Closed
wants to merge 1 commit into from

Conversation

jessesuen
Copy link
Member

To support PR promotions in kargo (with the render mechanism), kargo-render needs the ability to render to a target branch that is named differently than the path it is rendering from. This PR adds --ref-path so the two can be specified independently. If omitted, it preserves the original behavior of rendering from the path equal to --target-branch.

@jessesuen jessesuen requested a review from a team as a code owner December 21, 2023 09:51
Copy link

netlify bot commented Dec 21, 2023

Deploy Preview for docs-kargo-render-akuity-io canceled.

Name Link
🔨 Latest commit da313f5
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-render-akuity-io/deploys/65840aa354aa010009ae6178

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (1bdf2f6) 31.83% compared to head (da313f5) 31.74%.

Files Patch % Lines
service.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #228      +/-   ##
==========================================
- Coverage   31.83%   31.74%   -0.09%     
==========================================
  Files          14       14              
  Lines        1109     1112       +3     
==========================================
  Hits          353      353              
- Misses        728      731       +3     
  Partials       28       28              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krancour
Copy link
Member

Before merging this, I think we should discuss whether there are other options.

Two possibilities:

  • Kargo Render has the capability to open a PR itself and return information about it. We could take advantage of that.

  • If we didn't wish to do that for whatever reason (maybe we like how Kargo proper is opening PRs better than how Kargo Render does), then perhaps Kargo Render would benefit from an option that skips all commit / PR activity and writes rendered files to stdout or a specified location.

I think the second is my preference, because it eliminates some of the Kargo Render-specific code in Kargo proper.

Note that the second approach will tempt us to remove all commit / PR responsibility from Kargo Render and let it be responsible for rendering only, however, a disadvantage of that is it would limit Kargo Render's usefulness as a standalone tool or GitHub Action, so I wouldn't want to go that far.

@jessesuen
Copy link
Member Author

If we didn't wish to do that for whatever reason (maybe we like how Kargo proper is opening PRs better than how Kargo Render does), then perhaps Kargo Render would benefit from an option that skips all commit / PR activity and writes rendered files to stdout or a specified location.

I think the second is my preference

Great ideas! I really like the second option as well since in kargo proper I think I would like commits/prs to be done consistently. I'll close this PR and open an issue for 2nd approach.

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.

2 participants