fix: indentation in examples for git-open-pr.md #6273
Conversation
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Jesse Hitch <jesse.hitch@guerrilla-games.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6273 +/- ##
==========================================
+ Coverage 57.92% 57.96% +0.03%
==========================================
Files 496 496
Lines 41554 41599 +45
==========================================
+ Hits 24072 24113 +41
- Misses 16030 16032 +2
- Partials 1452 1454 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| config: | ||
| repoURL: https://github.com/example/repo.git | ||
| sourceBranch: ${{ outputs.push.branch }} | ||
| sourceBranch: ${{ task.outputs.push.branch }} |
There was a problem hiding this comment.
I am not supportive of this change. It wasn't wrong to begin with. There was simply an unstated assumption that the reader understood this snippet to have been extracted from a Stage's spec.promotionTemplate and not from a PromotionTask's or ClusterPromotionTask's spec.steps. The proposed change is only replacing one unstated assumption with a different unstated assumption. A better, (albeit much larger) fix would be to find all the examples throughout the docs where this unstated assumption exists and adjust those examples accordingly. i.e., this example becomes something like this:
steps:
apiVersion: kargo.akuity.io/v1alpha1
kind: Stage
# ...
spec:
# ...
promotionTemplate:
# Clone, prepare the contents of ./out, commit, etc...
- uses: git-push
as: push
config:
path: ./out
generateTargetBranch: true
- uses: git-open-pr
as: open-pr
config:
repoURL: https://github.com/example/repo.git
sourceBranch: ${{ outputs.push.branch }}
targetBranch: stage/${{ ctx.stage }}
# Wait for the PR to be merged or closed...
There was a problem hiding this comment.
Can we instead mention that tidbit somewhere? This was confusing for me and I lost an hour at work trying to figure out why it didn't work.
There was a problem hiding this comment.
In the meantime, I've reverted this to only be the indentation change.
There was a problem hiding this comment.
Can we instead mention that tidbit somewhere?
If you're proposing to mention it in one centralized place, I don't see any way that won't be overlooked.
It could be mentioned in plain English along with each example that might be prone to this confusion (any one that uses output from a previous step). That may end up being annoyingly repetitive, which would make establishing the context in the YAML as I proposed more attractive. Honestly, though, I worry even then could be overlooked.
Another option could be a comment anywhere output is used that says something like "# Or use task.outputs if using a (Cluster)PromotionTask."
In the meantime, I've reverted this to only be the indentation change.
Happy to merge this as-is.
There was a problem hiding this comment.
Created #6275 to track correction of the unstated assumption.
Signed-off-by: JesseBot <jessebot@linux.com>
|
Successfully created backport PR for |
All pull requests must reference an existing issue with no blocking labels.
PRs that do not meet this requirement will be automatically closed. See the
Contributor Guide for details.
Issue Reference
Closes #6272
Description
Fixes indentation in the git-open-pr docs.
Checklist
kind/proposal,needs discussion,needs research,maintainer only,area/security,size/large,size/x-large,size/xx-large).AI Use Disclosure
Select one:
Sign-Off
git commit -s) (required)git commit -S) (encouraged)