Skip to content

Conversation

@j1elo
Copy link
Contributor

@j1elo j1elo commented Jan 24, 2023

Why:

Closes #23465

What's being changed (if available, include any code snippets, screenshots, or gifs):

  1. Text mentioned a nonexistent key jobs.defaults.run.

  2. Text said that "You cannot use contexts or expressions in this keyword.", however the Context availability docs state that jobs.<job_id>.defaults.run should be able to access a variety of contexts. Empirically tested and the runner does in fact not fail when using the inputs context within a jobs.<job_id>.defaults.run entry.

  3. Removed extra repeated sentence after the link to defaults.run, leaving the context support of that key to be documented in that section.

Check off the following:

  • I have reviewed my changes in staging (look for the "Automatically generated comment" and click the links in the "Preview" column to view your latest changes).
  • For content changes, I have completed the self-review checklist.

@welcome
Copy link

welcome bot commented Jan 24, 2023

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 Jan 24, 2023
@j1elo
Copy link
Contributor Author

j1elo commented Jan 25, 2023

Only task remaining to be able to check off the requested checkboxes is to review the new contents in staging, but I do not know where I can find staging, it says to look for the "Automatically generated comment", but the only one is #23466 (comment) and it doesn't mention anything about staging so I'm a bit lost at this point.

@cmwilson21
Copy link
Contributor

@j1elo Thanks for submitting a PR and linking it to your issue ✨

Since this is a reusable, the automatically generated comment won't trigger. You can always run your branch locally to check for changes, but no worries on that for this content change as it won't hinder the review process to leave that box unchecked. I'll get it triaged for review! ⚡

@cmwilson21 cmwilson21 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 Jan 25, 2023
Copy link
Contributor

@skedwards88 skedwards88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix @j1elo ! Could you link to your workflow run logs that demonstrate that contexts and expressions can be used with these keys? That will help us review your PR. (And if you only tested contexts, should the text still mention that expressions are allowed?)

@cmwilson21 cmwilson21 added the more-information-needed More information is needed to complete review label Feb 9, 2023
@cmwilson21
Copy link
Contributor

👋 @j1elo Just wanted to check in and see if you've had a chance to link your workflow run logs as requested here. Feel free to ping me when you have! ✨

@hubwriter
Copy link
Contributor

I tested using a context:

jobs:
  dump_contexts_to_log:
    runs-on: ubuntu-latest
    defaults:
      run:
        shell: ${{ env.myshell }}

and an expression:

jobs:
  dump_contexts_to_log:
    runs-on: ubuntu-latest
    defaults:
      run:
        shell: ${{ format('ba{0}{1}', 's', 'h') }}

both of these work fine.

https://github.com/codspaces/hubwriter-javascript-action/blob/main/.github/workflows/context-test.yml

On the strength of this I think we're good to merge this PR.

@hubwriter
Copy link
Contributor

@j1elo - Many thanks for raising this PR, and the issue for it. 👍

I'm going to get this merged. 🚀

@hubwriter hubwriter enabled auto-merge September 15, 2023 11:23
@hubwriter hubwriter added this pull request to the merge queue Sep 15, 2023
Merged via the queue into github:main with commit 8f6d9b1 Sep 15, 2023
@github-actions
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

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 more-information-needed More information is needed to complete review waiting for review Issue/PR is waiting for a writer's review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong information in docs for defaults.run

4 participants