Skip to content

Fix actions skipped commit status indicator #34507

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion modules/structs/commit_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const (
CommitStatusFailure CommitStatusState = "failure"
// CommitStatusWarning is for when the CommitStatus is Warning
CommitStatusWarning CommitStatusState = "warning"
// CommitStatusSkipped is for when CommitStatus is Skipped
CommitStatusSkipped CommitStatusState = "skipped"
)

var commitStatusPriorities = map[CommitStatusState]int{
Copy link
Contributor

@ChristopherHX ChristopherHX May 23, 2025

Choose a reason for hiding this comment

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

Are commit statuses correctly merged with this change?

In Pull Request Status / Merged Commit Status Icon

I looked at other locations of the other constants.

e.g. assign skipped value 5

The compare method returns false in both directions if not part of commitStatusPriorities e.g. making a stable sort impossible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out.
Im not sure if semantically skipped is "better than" success, anyhow ill assign 5 to it and we will see if anyone else has feedback.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can read and read over the nobetterthan function, somehow I tend to get confused

This is a "no better than" table, negation.... Error has value 0, because it is a better status than warning, pending, success.
e.g. if we start with skipped/success, we end with error if a single status is error

After reading my suggestion here seems to be not required, but I found something else about PRs.
NoBetterThan is never used inverted and having no value defined for skipped would have a similar outcome if every other value is already in this table.

Expand All @@ -26,6 +28,7 @@ var commitStatusPriorities = map[CommitStatusState]int{
CommitStatusWarning: 2,
CommitStatusPending: 3,
CommitStatusSuccess: 4,
CommitStatusSkipped: 5,
}

func (css CommitStatusState) String() string {
Expand All @@ -35,7 +38,7 @@ func (css CommitStatusState) String() string {
// NoBetterThan returns true if this State is no better than the given State
// This function only handles the states defined in CommitStatusPriorities
func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
// NoBetterThan only handles the 5 states above
// NoBetterThan only handles the 6 states above
if _, exist := commitStatusPriorities[css]; !exist {
return false
}
Expand Down
4 changes: 3 additions & 1 deletion services/actions/commit_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,14 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er

func toCommitStatus(status actions_model.Status) api.CommitStatusState {
switch status {
case actions_model.StatusSuccess, actions_model.StatusSkipped:
case actions_model.StatusSuccess:
return api.CommitStatusSuccess
case actions_model.StatusFailure, actions_model.StatusCancelled:
return api.CommitStatusFailure
case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning:
return api.CommitStatusPending
case actions_model.StatusSkipped:
return api.CommitStatusSkipped
default:
return api.CommitStatusError
}
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/commit_status.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@
{{if eq .State "warning"}}
{{svg "gitea-exclamation" 18 "commit-status icon text yellow"}}
{{end}}
{{if eq .State "skipped"}}
{{svg "octicon-skip" 18 "commit-status icon text grey"}}
{{end}}
3 changes: 2 additions & 1 deletion web_src/js/components/DashboardRepoList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {fomanticQuery} from '../modules/fomantic/base.ts';

const {appSubUrl, assetUrlPrefix, pageData} = window.config;

type CommitStatus = 'pending' | 'success' | 'error' | 'failure' | 'warning';
type CommitStatus = 'pending' | 'success' | 'error' | 'failure' | 'warning' | 'skipped';

type CommitStatusMap = {
[status in CommitStatus]: {
Expand All @@ -22,6 +22,7 @@ const commitStatus: CommitStatusMap = {
error: {name: 'gitea-exclamation', color: 'red'},
failure: {name: 'octicon-x', color: 'red'},
warning: {name: 'gitea-exclamation', color: 'yellow'},
skipped: {name: 'octicon-skip', color: 'grey'},
};

export default defineComponent({
Expand Down