-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fix Workflow names instead of workflow file name #31474
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
base: main
Are you sure you want to change the base?
Conversation
{{- end -}} | ||
{{$CurWorkflow := $.CurWorkflow}} | ||
{{$WorkflowName := .Workflow.Name}} | ||
{{with .ActionRun}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable shadowing is more complex now. And the name Run
also seems ambiguous
I think it's better to do so:
{{range $run = .WorkflowActionRuns}}
{{$actionRun := $run.ActionRun}}
{{$workflow = $run.Workflow}}
{{$actionRun.Title}}
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the idea, I did the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I tested in GitHub actions and they show the full path to the file when workflow name is absent, e.g. |
IIRC, GitHub has some bugs of display workflow name. That’s why no one fix this issue for a long time I think. |
Do you have any idea which special cases I should check or where I should look for them? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per @yp05327, we should probably iron out edge cases first
Stale for long time |
Hey! Just wondering what's the state of this? Also, it would be great if the name of the workflow could also be inside the workflow run page. Many thanks! |
Seems like OP is unresponsive. Maybe someone should open another PR based on this one with the remarks from #31474 (comment) adressed. |
@ghnp5 I think there is a blocker for this change.
Hmm, I think I could understand the problem more now. And yes, that's the key problem.
The workflow information is only read from default branch. Maybe we need to document the workflow behavior first (or are there already some documents we can refer?) |
IIRC, GitHub also has some issues when handling the workflow name from different branches. (by my test, about 1 year ago? maybe they have fixed it or not)
So I strongly agree. Design the handling of workflow names in different branches with same name or maybe other special cases first.
Then coding, and add tests to confirm whether it works well in this cases. |
Some tests can be made in GitHub and see how it behaves, for those cases. But, I was thinking something simple, like:
In most cases, this will be consistent across all branches, and if for some reason one branch is going to update the name of the workflow, it will eventually be merged to the default branch, and the name will be updated when that happens. |
How to define |
You're right to ask that. What I had in mind is that "it doesn't matter", so whatever order Gitea does now, for looping through the branches, could be kept :) So in other words, "no order guaranteed", I was thinking. Just "random/whatever". But if an order has to be defined, then I think "the latest updated branch" would make more sense. |
Yeah, from user side, some users may not even have this problem or they don't care about the order or something else, but the developers should do their best to cover all cases, IMO. |
Fixes #31458 and maybe #25912
I added the name from the YAML file to the workflow object. If there is no name in the file, the name remains the filename.
I created an object to connect the run and workflow together, which was necessary to show workflow name in action run list.
(Easier to review with disabled whitespace changes, unfortunately there is a lot of them in runs_list.tmpl :( )