-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fix: Add appendTemplatedValues to pullRequest generator (#11408) #21110
base: master
Are you sure you want to change the base?
Conversation
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
5e28575
to
efb5e26
Compare
Signed-off-by: Brett C. Dudo <[email protected]>
Signed-off-by: Brett C. Dudo <[email protected]>
Signed-off-by: Brett C. Dudo <[email protected]>
Signed-off-by: Brett C. Dudo <[email protected]>
6fb7735
to
9a44fa1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21110 +/- ##
==========================================
+ Coverage 55.16% 55.20% +0.03%
==========================================
Files 324 324
Lines 55603 55606 +3
==========================================
+ Hits 30674 30695 +21
+ Misses 22303 22289 -14
+ Partials 2626 2622 -4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I don't think we need to do any checks here because they're all done on the values anyway so I don't see any issues with that.
Thanks for the PR 💪
I can't seem to run ERROR: failed to solve: docker.io/library/golang:1.23.4@sha256:a5ec4a1403fb63b1afc4643d707a4ee11ab4b4637fb73afa3f05ee67b9282c92: failed to resolve source metadata for docker.io/library/golang:1.23.4@sha256:a5ec4a1403fb63b1afc4643d707a4ee11ab4b4637fb73afa3f05ee67b9282c92: no match for platform in manifest: not found |
You got it, I'll see if I can do that tomorrow. |
This isn't an apple silicon issue, it looks like that specific image version is just missing for some reason. I updated it and ran locally but this should be merged separately. Opening a PR now. |
New PR #21174 |
That PR helped, thanks! I'm struggling with permissions, though, which seems odd considering this happens within a container. There's a slew of these errors:
I'm running
Is there some env that I'm missing? Or an option in the command? |
Does this overlap with this other PR ? #16507 |
It does, @lukepatrick ! And he beat me by a week 😅 As much as I'd like to be a contributor, I say we just grab the specs from this PR, and get @crenshaw-dev 's PR over the finish line in time for the RC. |
A week+1year. Either look good, I would vote for yours, likely no rebase challenges like the other, and all the more contributors the better. |
OMG... I completely glossed over the year, lol. I'm happy to push this one to the finish line. I'm struggling to get |
Fixes #11408
This seems as though it'd be something that we'll want to roll out to all the generators. It seems more flexible than adding something like
branchParamPrefix
, and less disruptive than adding a new key forpr_branch
which is superfluous, and may end up being deprecated at some point in the future.If there's appetite to add it to the rest of the generators, I'm happy to add that to the PR, but it's beyond the scope of the linked issue.
Checklist: