docs icon indicating copy to clipboard operation
docs copied to clipboard

Concurrency examples are inconsistent with English text around them

Open strugee opened this issue 1 year ago • 6 comments

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 avatar Mar 07 '24 04:03 strugee

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.

welcome[bot] avatar Mar 07 '24 04:03 welcome[bot]

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

nguyenalex836 avatar Mar 07 '24 18:03 nguyenalex836

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

erta

FR4NKWest avatar Mar 09 '24 13:03 FR4NKWest

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 avatar Mar 11 '24 18:03 jc-clark

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

strugee avatar Mar 11 '24 21:03 strugee

A stale label has been added to this issue because it has been open for 60 days with no activity. To keep this issue open, add a comment within 3 days.

github-actions[bot] avatar May 20 '24 16:05 github-actions[bot]