Skip to content

Fix ListWorkflowRuns OpenAPI response model. #35026

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 8 commits into from
Jul 10, 2025

Conversation

ScionOfDesign
Copy link
Contributor

Change the OpenAPI response of ListWorkflowRuns to WorkflowRunsList like it is supposed to be.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 10, 2025
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Jul 10, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 10, 2025
@ScionOfDesign
Copy link
Contributor Author

Hold off for a bit, I just encountered another error when actually trying to deserialize this endpoint to WorkflowRunsList.

@wxiaoguang wxiaoguang marked this pull request as draft July 10, 2025 02:13
@ScionOfDesign
Copy link
Contributor Author

It's odd, in the response both licenses and repo_transfer are being returned as null, but they are required in the OpenAPI spec:

        "has_wiki": true,
        "has_pull_requests": true,
        "has_projects": true,
        "projects_mode": "all",
        "has_releases": true,
        "has_packages": true,
        "has_actions": true,
        "ignore_whitespace_conflicts": false,
        "allow_merge_commits": true,
        "allow_rebase": true,
        "allow_rebase_explicit": true,
        "allow_squash_merge": true,
        "allow_fast_forward_only_merge": true,
        "allow_rebase_update": true,
        "allow_manual_merge": false,
        "autodetect_manual_merge": false,
        "default_delete_branch_after_merge": false,
        "default_merge_style": "merge",
        "default_allow_maintainer_edit": false,
        "avatar_url": "",
        "internal": false,
        "mirror_interval": "",
        "object_format_name": "sha1",
        "mirror_updated": "0001-01-01T00:00:00Z",
        "repo_transfer": null,
        "topics": [],
        "licenses": null

@ScionOfDesign
Copy link
Contributor Author

Fixed the model to omit null (but not nullable) values and initialize the Licenses array in the response even if they are empty.

@ScionOfDesign ScionOfDesign marked this pull request as ready for review July 10, 2025 04:26
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 10, 2025
@ScionOfDesign
Copy link
Contributor Author

Forgot that the original didn't have the pre-allocation. Your suggestion should be ideal.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jul 10, 2025

Forgot that the original didn't have the pre-allocation. Your suggestion should be ideal.

For performance-critical code, that if and pre-allocation does help.

While here I think we can keep StringList as simple as before. TBH I think in this case there should be no noticeable difference, while simple code reads better.


(I just meant that in the convert function, we can't depend on the StringList's behavior)

@wxiaoguang wxiaoguang enabled auto-merge (squash) July 10, 2025 05:31
@wxiaoguang wxiaoguang added backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.24 This PR should be backported to Gitea 1.24 labels Jul 10, 2025
@wxiaoguang wxiaoguang removed backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.24 This PR should be backported to Gitea 1.24 labels Jul 10, 2025
@wxiaoguang wxiaoguang merged commit af0196c into go-gitea:main Jul 10, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.25.0 milestone Jul 10, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 10, 2025
* giteaofficial/main:
  Fix ListWorkflowRuns OpenAPI response model. (go-gitea#35026)
  Partially refresh notifications list (go-gitea#35010)
@ScionOfDesign ScionOfDesign deleted the fix-workflow-runs-openapi branch July 10, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants