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

Concurrency examples are inconsistent with English text around them #31988

Open
1 task done
strugee opened this issue Mar 7, 2024 · 6 comments
Open
1 task done

Concurrency examples are inconsistent with English text around them #31988

strugee opened this issue Mar 7, 2024 · 6 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

Comments

@strugee
Copy link
Contributor

strugee commented Mar 7, 2024

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency

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

This section is just generally very confusing; see this issue's title. Every example uses group: and cancel-in-progress:. However, the latter isn't even supposed to be introduced - AFAICT - until the section labeled "Example: Using concurrency to cancel any in-progress job or run". It's extremely confusing that the first example uses both of these keys but they haven't been introduced yet. As another example, one of the examples talks about specifying concurrency: ci-${{ github.ref }}, and then the following example does not use this expression - it uses group: ci-${{ github.ref }}.

Additional information

No response

@strugee strugee added the content This issue or pull request belongs to the Docs Content team label Mar 7, 2024
Copy link

welcome bot commented Mar 7, 2024

Thanks for opening this issue. 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 Mar 7, 2024
@nguyenalex836
Copy link
Contributor

@strugee Thanks for opening this issue! I'll get this triaged for review ✨

@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 Mar 7, 2024
@FR4NKWest

This comment was marked as spam.

@jc-clark
Copy link
Contributor

jc-clark commented Mar 11, 2024

Thanks for the feedback @strugee. We took a look into the issues you raised. A couple considerations/ideas:

Cancel in progress

This section is just generally very confusing; see this issue's title. Every example uses group: and cancel-in-progress:. However, the latter isn't even supposed to be introduced - AFAICT - until the section labeled "Example: Using concurrency to cancel any in-progress job or run". It's extremely confusing that the first example uses both of these keys but they haven't been introduced yet.

A paragraph immediately before the examples (under the "concurrency" header) does explain cancel-in-progress.

To also cancel any currently running job or workflow in the same concurrency group, specify cancel-in-progress: true. To conditionally cancel currently running jobs or workflows in the same concurrency group, you can specify cancel-in-progress as an expression with any of the allowed expression contexts.

So, in this sense, the cancel-in-progress concepts used in the examples are already introduced.

Since this concept is covered in the overview paragraphs, maybe it makes most sense to completely delete the "Example: Using concurrency to cancel any in-progress job or run" section.

Expression

As another example, one of the examples talks about specifying concurrency: ci-${{ github.ref }}, and then the following example does not use this expression - it uses group: ci-${{ github.ref }}.

In the example you highlight, group is nested under the concurrency key. So, I believe the intention of this example is mainly to highlight that you can use the ci-${{ github.ref }} expression. Perhaps it would make sense to update the sentence as follows:

change:

using a dynamic expression such as concurrency: ci-${{ github.ref }} in your workflow

to:

using a dynamic expression such as ci-${{ github.ref }} within the concurrency key in your workflow

@jc-clark jc-clark removed the waiting for review Issue/PR is waiting for a writer's review label Mar 11, 2024
@strugee
Copy link
Contributor Author

strugee commented Mar 11, 2024

In the example you highlight, group is nested under the concurrency key. So, I believe the intention of this example is mainly to highlight that you can use the ci-${{ github.ref }} expression.

Ok, sure, but if you make the suggested change now the problem is that concurrency: <some value or expression> isn't documented anywhere. I came to these docs having already seen that construct in the wild when I couldn't confirm from the docs my guess that these two snippets are semantically equivalent:

concurrency: foo
concurrency:
  group: foo

So, in this sense, the cancel-in-progress concepts used in the examples are already introduced.

Fair enough, I must've misremembered or something. But I still think the example would be clearer if it wasn't included, because it doesn't have anything to do with "Using concurrency and the default behavior".

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
Projects
None yet
Development

No branches or pull requests

6 participants
@strugee @jc-clark @nguyenalex836 @FR4NKWest and others