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

Docker pre.entrypoint links to Node pre.pre-if #35264

Open
1 task done
jsoref opened this issue Nov 12, 2024 · 8 comments
Open
1 task done

Docker pre.entrypoint links to Node pre.pre-if #35264

jsoref opened this issue Nov 12, 2024 · 8 comments
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

Comments

@jsoref
Copy link
Contributor

jsoref commented Nov 12, 2024

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#runspre-entrypoint

What part(s) of the article would you like to see updated?

I think a distinct .pre-if should be added to the https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#runs-for-docker-container-actions and https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#runspre-entrypoint's runs.pre-if link should be changed to point to it.

Additional information

Here's a run with pre-if: true:

  pre-entrypoint: '/pre.sh'
  pre-if: true

pre

Here's a run with pre-if: false (which is good because the pre-entrypoint was broken):

  pre-entrypoint: 'pre.sh'
  pre-if: false

https://github.com/check-spelling-sandbox/bookish-rotary-phone/actions/runs/11798769996/job/32865715637

@jsoref jsoref added the content This issue or pull request belongs to the Docs Content team label Nov 12, 2024
@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Nov 12, 2024
@jsoref
Copy link
Contributor Author

jsoref commented Nov 12, 2024

I'm not sure how easy it would be to split the two pre-if statements two get distinct anchors. Offhand, I'd strongly recommend splitting the document into:

  1. page for actions in general that links to individual pages for javascript, composite, and docker
  2. reusable shared-action-definitions head basically everything from https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#about-yaml-syntax-for-github-actions stopping before runs
  3. page for JavaScript actions with includes for reusable shared-action-definitions head and reusable shared-action-definitions tail
  4. page for composite actions with includes for reusable shared-action-definitions head and reusable shared-action-definitions tail
  5. page for Docker container actions with includes for reusable shared-action-definitions head and reusable shared-action-definitions tail
  6. reusable shared-action-definitions tail with branding

The number of people who want to look at the syntax for more than one kind of action at a time can be counted on one hand and they all more or less work on either actions/runner or nektos/act. Everyone else only cares about one kind of action (on average javascript or composite) and doesn't need to be confused by the other two.

@nguyenalex836 nguyenalex836 added 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 Nov 12, 2024
@nguyenalex836
Copy link
Contributor

@jsoref Thank you for raising this issue! I'll get this triaged for review ✨ Our team will provide feedback regarding the best next steps for this issue - thanks for your patience! 💛

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

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

@nguyenalex836
Copy link
Contributor

@jsoref Thanks for your patience while our SME team reviewed! ✨ They agreed on almost suggestions, but wanted to relay the following -

The only thing is that I don't think that a new section like this should just link to the run.pre-if section for Javascript, because of the disparity between pre vs pre-entrypoint.

We wanted to get your thoughts on this before opening this up for contributions, just so we're all on the same page 💛

@jsoref
Copy link
Contributor Author

jsoref commented Nov 26, 2024

I'm happy with totally different content sections or reusable content blocks. I can't remember precisely what I was thinking here and the GitHub Mobile app UI is not helping me figure it out.

As long as it's possible to easily understand the two at least somewhat unrelated items, I'd be happy.

(I don't expect to be able to look back/into anything until late December or early January.)

@nguyenalex836
Copy link
Contributor

I'm happy with totally different content sections or reusable content blocks. I can't remember precisely what I was thinking here and the GitHub Mobile app UI is not helping me figure it out.

As long as it's possible to easily understand the two at least somewhat unrelated items, I'd be happy.

@jsoref Awesome! We're aligned on this ✨

(I don't expect to be able to look back/into anything until late December or early January.)

No problem - we are happy to let this issue rest until you are ready to submit a PR if this sounds good to you 💛

@jsoref
Copy link
Contributor Author

jsoref commented Nov 26, 2024

Feel free to update the guidance and I'll try to circle back when I can.

@jsoref
Copy link
Contributor Author

jsoref commented Nov 26, 2024

I should note that I don't think I was really sure how to make two sections with the same name.

I think somewhere I had a note / idea that the action documentation should be split into distinct files based on the category. There's too little value in showing a composite author information about docker action or JavaScript.

Where the content is the same, using reusables seems like a better strategy than sharing the same user facing web page.

@nguyenalex836 nguyenalex836 added 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 Nov 26, 2024
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

No branches or pull requests

3 participants