Skip to content

Keeping consistent between UI and API about combined commit status state and fix some bugs #34562

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 7 commits into
base: main
Choose a base branch
from

Conversation

lunny
Copy link
Member

@lunny lunny commented May 28, 2025

Extract from #34531

Move Commit status state to a standalone package

Move the state from structs to commitstatus package. It also introduce CommitStatusStates so that the combine function could be used from UI and API logic.

Combined commit status Changed

This PR will follow Github's combined commit status. Before this PR, every commit status could be a combined one.
According to https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#get-the-combined-status-for-a-specific-reference

Additionally, a combined state is returned. The state is one of:
failure if any of the contexts report as error or failure
pending if there are no statuses or a context is pending
success if the latest status for all contexts is success

This PR will follow that rule and remove the NoBetterThan logic. This also fixes the inconsistent between UI and API. In the API convert package, it has implemented this which is different from the UI. It also fixed the missing URL and CommitURL in the API.

CalcCommitStatus return nil if there is no commit statuses

The behavior of CalcCommitStatus is changed. If the parameter commit statuses is empty, it will return nil. The reference places should check the returned value themselves.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 28, 2025
@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label May 28, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/api This PR adds API routes or modifies them labels May 28, 2025
@lunny lunny force-pushed the lunny/commit_status_refactor branch from 7f22185 to 1ad800d Compare May 30, 2025 23:23
@lunny lunny marked this pull request as ready for review May 31, 2025 18:33
@lunny lunny changed the title Refactor commit status state Keeping consistent between UI and API about comibined commit status state and fix some bugs May 31, 2025
@lunny lunny changed the title Keeping consistent between UI and API about comibined commit status state and fix some bugs Keeping consistent between UI and API about combined commit status state and fix some bugs May 31, 2025
@lunny lunny added the type/bug label May 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants