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

Add context information for jobs.<job_id>.uses #34833

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

lachieh
Copy link
Contributor

@lachieh lachieh commented Oct 4, 2024

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:

  • I have reviewed my changes in staging, available via the View deployment link in this PR's timeline (this link will be available after opening the PR).
    • For content changes, you will also see an automatically generated comment with links directly to pages you've modified. The comment won't appear if your PR only edits files in the data directory.
  • For content changes, I have completed the self-review checklist.

Copy link

welcome bot commented Oct 4, 2024

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.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Oct 4, 2024
Copy link
Contributor

github-actions bot commented Oct 4, 2024

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 that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.


Content directory changes

You 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.

Source Preview Production What Changed
actions/writing-workflows/choosing-what-your-workflow-does/accessing-contextual-information-about-workflow-runs.md fpt
ghec
ghes@ 3.14 3.13 3.12 3.11 3.10
fpt
ghec
ghes@ 3.14 3.13 3.12 3.11 3.10
actions/sharing-automations/reusing-workflows.md fpt
ghec
ghes@ 3.14 3.13 3.12 3.11 3.10
fpt
ghec
ghes@ 3.14 3.13 3.12 3.11 3.10
from reusable
actions/writing-workflows/workflow-syntax-for-github-actions.md fpt
ghec
ghes@ 3.14 3.13 3.12 3.11 3.10
fpt
ghec
ghes@ 3.14 3.13 3.12 3.11 3.10
from reusable

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server

@nguyenalex836 nguyenalex836 added content This issue or pull request belongs to the Docs Content team actions This issue or pull request should be reviewed by the docs actions team waiting for review Issue/PR is waiting for a writer's review and removed triage Do not begin working on this issue until triaged by the team labels Oct 4, 2024
@nguyenalex836
Copy link
Contributor

@lachieh Thanks so much for opening a PR! I'll get this triaged for review ✨

@subatoi subatoi added the needs SME This proposal needs review from a subject matter expert label Oct 7, 2024
Copy link
Contributor

github-actions bot commented Oct 7, 2024

Thanks for opening a pull request! We've triaged this issue for technical review by a subject matter expert 👀

@nguyenalex836
Copy link
Contributor

@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:

The following table indicates where each context and special function can be used within a workflow. Unless listed below, a function can be used anywhere.

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.
Perhaps something like this to replace that would be a better option:

The following table lists the restrictions on where each context and special function can be used within a workflow. The listed contexts are only available for the given workflow key, and may not be used anywhere else. Unless listed below, a function can be used anywhere.


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 💛

@nguyenalex836 nguyenalex836 added more-information-needed More information is needed to complete review SME reviewed An SME has reviewed this issue/PR and removed waiting for review Issue/PR is waiting for a writer's review needs SME This proposal needs review from a subject matter expert labels Oct 18, 2024
@LucifeMORNINGSA84

This comment was marked as spam.

LucifeMORNINGSA84

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.
@lachieh
Copy link
Contributor Author

lachieh commented Oct 20, 2024

@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 defaults.run. Because of that, I've copied the sentence from that section into the section for jobs.<job-id>.uses. Happy to remove this change if needed.

@nguyenalex836 nguyenalex836 removed the more-information-needed More information is needed to complete review label Oct 21, 2024
@nguyenalex836 nguyenalex836 added this pull request to the merge queue Oct 21, 2024
Merged via the queue into github:main with commit 1ff44fe Oct 21, 2024
45 checks passed
Copy link
Contributor

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

@nguyenalex836
Copy link
Contributor

@lachieh I had a messaged queued up, but forgot to send before I merged your PR!

Because of that, I've copied the sentence from that section into the section for jobs..uses.

I think this is a fair addition 💛 thanks again for your contribution! ✨

@lachieh
Copy link
Contributor Author

lachieh commented Oct 21, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team SME reviewed An SME has reviewed this issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants