Skip to content

gh-124471: Set name for unnamed reusable workflow #124475

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

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Sep 24, 2024

@hugovk
Copy link
Member

hugovk commented Sep 25, 2024

Please can you share a before and after screenshot of what actually changed in the UI?

@corona10
Copy link
Member Author

Screenshot from 2024-09-24 20-15-02

@hugovk Can you see it how it is changed?

@hugovk
Copy link
Member

hugovk commented Sep 25, 2024

Yes, now I see. Here is how it looks now (at https://github.com/python/cpython/actions):

image

Although I'm not sure if we want these actual names, because if you click the "macOS" one, you don't see anything useful:

image

These are reusable workflows, which are like include files for the main "Tests" build. People might be confused why they don't see the macOS builds here.

@corona10
Copy link
Member Author

corona10 commented Sep 25, 2024

Although I'm not sure if we want these actual names, because if you click the "macOS" one, you don't see anything useful:

Agree, then we have to leave as-is or define somekinds special naming that people notice that it's not a actual workflow.
(e.g, [reusable workflow] macOS something like that).

@webknjaz
Copy link
Contributor

webknjaz commented Oct 9, 2024

@hugovk could you go through all of the reusable workflows on the https://github.com/python/cpython/actions page, click on each ending up on a dedicated page (like https://github.com/python/cpython/actions/workflows/reusable-macos.yml), then use the three-dots top right menu to disable them? This will not affect their reusability, the calling workflows will still be able to invoke them. But the UI will show them as disabled. Here's an example of a project set up like this: https://github.com/ansible/awx-plugins/actions. The last one in the left sidebar is marked as Disabled.

Copy link
Contributor

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

I was solving a similar concern while setting up new projects recently. I ended up putting a bit red cross with a “do not click” message in mine. I also marked the reusable workflows as disabled, so they are always at the bottom of the list.

Here's a demo of what it looks like in practice: https://github.com/ansible/awx-plugins/actions.

@hugovk would you like something similar here? I've copied my approach to the suggested changes but perhaps you'd want something shorter. LMK.

@webknjaz
Copy link
Contributor

webknjaz commented Oct 9, 2024

define somekinds special naming that people notice that it's not a actual workflow.

If you go this way, and I think you should, I suggest also applying this to other reusable-*.yml workflow files. I realized that I've already added names to some of those, but it'd be nice to make them consistent in the UI, just like the filename prefix convention I introduced in the past.

@webknjaz
Copy link
Contributor

webknjaz commented Oct 9, 2024

Interestingly, I noticed 2 different workflows marked as Docs in the sidebar on the Actions tab. I was surprised, since I remember deleting one when introducing the reusable approach. So I clicked on the one that's not reusable and realized that it's coming from the 3.10 branch 🤯

This is to say that marking the workflows as reusable explicitly in their names will be helpful.

@AA-Turner AA-Turner closed this Oct 9, 2024
@AA-Turner AA-Turner reopened this Oct 9, 2024
@AA-Turner
Copy link
Member

The workflows are now marked "disabled"; closing/reopening this PR to check that they are all still invoked.

@AA-Turner
Copy link
Member

AA-Turner commented Oct 9, 2024

perhaps you'd want something shorter

Let's avoid emoji, and perhaps just using "Reusable" will signal that they're special -- the "DO NOT CLICK" is a little distracting personally (albeit explicit).

A

@webknjaz
Copy link
Contributor

The workflows are now marked "disabled"

@AA-Turner FYI, you missed a few like https://github.com/python/cpython/actions/workflows/reusable-docs.yml which was less obvious since it actually has a name (so it'll also need to be renamed following the same pattern).

Copy link
Contributor

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

perhaps you'd want something shorter

Let's avoid emoji, and perhaps just using "Reusable" will signal that they're special -- the "DO NOT CLICK" is a little distracting personally (albeit explicit).

I suspected that people would not want emojis. The reason I started using them is that they are meant to be visually discouraging.

@hugovk @AA-Turner let me know what you want exactly, and I can update the suggested changes accordingly. Just Reusable {{ X }} or anything else?

@webknjaz
Copy link
Contributor

The workflows are now marked "disabled"

@AA-Turner FYI, you missed a few like python/cpython/actions/workflows/reusable-docs.yml which was less obvious since it actually has a name (so it'll also need to be renamed following the same pattern).

I've found two more: https://github.com/python/cpython/actions/workflows/reusable-windows-msi.yml / https://github.com/python/cpython/actions/workflows/reusable-change-detection.yml.

All three should update their names to whatever convention ends up being decided on in this PR.

@AA-Turner
Copy link
Member

let me know what you want exactly, and I can update the suggested changes accordingly. Just Reusable {{ X }} or anything else?

Either Reusable {{ X }} or Reusable: {{ X }} are fine by me.

A

@webknjaz
Copy link
Contributor

Then just accept my variant 2 suggestions and wait for @corona10 to additionally make these changes to 3 more files, I suppose.

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@corona10
Copy link
Member Author

Hmm if there is no more objection,let's merge this PR?

@AA-Turner AA-Turner enabled auto-merge (squash) October 10, 2024 12:14
@webknjaz
Copy link
Contributor

@AA-Turner perhaps also add backport request labels?

@AA-Turner AA-Turner added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 10, 2024
@AA-Turner AA-Turner merged commit e4cab48 into python:main Oct 10, 2024
40 checks passed
@miss-islington-app
Copy link

Thanks @corona10 for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 10, 2024
)

(cherry picked from commit e4cab48)

Co-authored-by: Donghee Na <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@miss-islington-app
Copy link

Sorry, @corona10 and @AA-Turner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e4cab488d4445e8444932f3bed1c329c0d9e5038 3.12

@miss-islington-app miss-islington-app bot assigned AA-Turner and unassigned hugovk Oct 10, 2024
@bedevere-app
Copy link

bedevere-app bot commented Oct 10, 2024

GH-125256 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 10, 2024
@bedevere-app
Copy link

bedevere-app bot commented Oct 10, 2024

GH-125257 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 10, 2024
corona10 added a commit to corona10/cpython that referenced this pull request Oct 10, 2024
…nGH-124475)

(cherry picked from commit e4cab48)

Co-authored-by: Donghee Na <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@hugovk
Copy link
Member

hugovk commented Oct 10, 2024

Thanks all!

corona10 added a commit that referenced this pull request Oct 10, 2024
…h-125257)

(cherry picked from commit e4cab48)

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
corona10 added a commit that referenced this pull request Oct 10, 2024
…h-125256)

gh-124471: Set name for unnamed reusable workflow (GH-124475)
(cherry picked from commit e4cab48)

Co-authored-by: Donghee Na <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants