-
Notifications
You must be signed in to change notification settings - Fork 60.5k
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
Add context information for jobs.<job_id>.uses
#34833
Conversation
Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
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 |
@lachieh Thanks so much for opening a PR! I'll get this triaged for review ✨ |
Thanks for opening a pull request! We've triaged this issue for technical review by a subject matter expert 👀 |
@lachieh Thank you for your patience while our SME team reviewed! ✨ They responded with the following: I don't think this is the right place to convey this information. We don't list where contexts cannot be used in the availability table, we only list where they can be used. Right above that table we state:
So this is the part that could be clearer to me. We have two things: contexts and functions. In the table, the contexts column lists the only places where the given contexts can be used. However, the functions column only lists the places where either 1) Functions cannot be used at all or 2) Only a specific set of functions can be used. So unless that's clear to the reader going in they might have the wrong expectation.
Would you be open to updating your PR to make the changes our SME team suggested instead? We'll be happy to merge this PR once that is finished 💛 |
This comment was marked as spam.
This comment was marked as spam.
As far as I can tell, this property does not have access to any context and needs to be a statically defined string. If this is incorrect, it'd be great to have some clarification here.
@nguyenalex836 Thanks for the review. I made the change to the text before the table, and removed the added row. I do feel that it is still not quite clear, as neither functions nor context appear to be usable in this key. The same restrictions apply in many other places, such as |
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 ⚡ |
@lachieh I had a messaged queued up, but forgot to send before I merged your PR!
I think this is a fair addition 💛 thanks again for your contribution! ✨ |
Thank you! |
Why:
As far as I can tell, this property does not have access to any context and needs to be a statically defined string. If this is incorrect, it'd be great to have some clarification here.
What's being changed (if available, include any code snippets, screenshots, or gifs):
Add row for
jobs.<job_id>.uses
into context availability table. Alternatively, a note at the top of this section could be used instead saying indicating that if the key is not listed in the table, it does not/may not support expression evaluation from context data.As a third option, a note about this behavior could be added to the relevant section on the workflow syntax page.
Check off the following:
data
directory.