-
Notifications
You must be signed in to change notification settings - Fork 58.8k
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 regression in artifact-attestations-step-for-container-images.md #33917
Conversation
Fixes regression where the reusable artifact attestations step example was accidentally broken by a commit that replaced dashes with asterisks. Commit that broke example: 0886a39 Co-authored-by: snorremd <snorremd@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Automatically generated comment ℹ️This comment is automatically generated and will be overwritten every time changes are committed to this branch. The table contains an overview of files in the Content directory changesYou may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.
fpt: Free, Pro, Team |
Hi @janbrasna—in hindsight, I think a better solution would probably be to just make these not-reusables, as there's only three instances of it anyway. I'm sorry for misleading you. Let me run it by our Engineering team to see what they think—when we open a PR to fix it, whatever the fix looks like, I'll make sure you're added as co-contributor so you receive the credit, if that sounds OK? |
@subatoi Sure, whatever feels more stable for now. I've updated this PR to getting rid of the reusable (which is not a bad idea after all, as it is probably the only occurrence where bits of code are placed in toplevel markdown file without any guardrails around defining what formatting/markup it's supposed be written in, a footgun in itself…) Please don't bother with credits et al., I only wanted to get this moving — whatever path you end up picking feel free to just close this out, no worries;) |
This comment was marked as spam.
This comment was marked as spam.
2 similar comments
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
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.
Thanks for catching this and working with us to fix it, @janbrasna! I think the workaround you added looks good, so I'll go ahead and get this merged.
Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues ⚡ |
This is another approach to #33793
Fixes #33794