Skip to content

Correct swagger annotations for enums, status codes, and notification state#37030

Merged
wxiaoguang merged 2 commits intogo-gitea:mainfrom
myers:fix/swagger-annotations
Mar 30, 2026
Merged

Correct swagger annotations for enums, status codes, and notification state#37030
wxiaoguang merged 2 commits intogo-gitea:mainfrom
myers:fix/swagger-annotations

Conversation

@myers
Copy link
Copy Markdown
Contributor

@myers myers commented Mar 29, 2026

⚠️ BREAKING ⚠️

  • delete reaction endpoints is changed to return 204 No Content rather than 200 with no content.

Summary

Add swagger:enum annotations and migrate all enum comments from the deprecated comma-separated format to JSON arrays. Introduce NotifySubjectStateType with open/closed/merged values. Fix delete reaction endpoints to return 204 instead of 200.

This is one part of a stacked set. The other adds a OpenAPI 3 spec generated off of the swagger spec. With these corrections it is good enough to have OpenAPI 3 code generating tools (like rust's https://github.com/oxidecomputer/progenitor) to create API clients based off of it.

This was built in Claude Code, with human review.

… state

Add swagger:enum annotations and migrate all enum comments from the
deprecated comma-separated format to JSON arrays. Introduce
NotifySubjectStateType with open/closed/merged values. Fix delete
reaction endpoints to return 204 instead of 200.

BREAKING: delete reaction endpoints now return 204 No Content.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 29, 2026
@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 Mar 29, 2026
@GiteaBot GiteaBot added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Mar 29, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

This change seems good.


For the plan (https://github.com/myers/gitea/tree/feat/openapi3-conversion, especially the commit myers@14d2832), I recalled some discussions for this topic:

(als cc @TheFox0x7 🙏 )

Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 left a comment

Choose a reason for hiding this comment

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

+1 from me. I don't see anything wrong with this.

@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 Mar 29, 2026
@TheFox0x7
Copy link
Copy Markdown
Contributor

As for the v3; I don't mind generating v3 from v2 if you want to send that too. Though I'd rather generate v2 from v3 and get rid of generating spec from comments as - as you can see - it's not accurate at times, there are not examples etc.
Generating this form I/O structs like huma does it would be better but integrating that would take a while (especially to review later).

I won't even suggest writing spec first and generating server from that as I don't want to do it myself :)

@wxiaoguang
Copy link
Copy Markdown
Contributor

For "v3", some questions in my mind:

  • Should it co-exist with v2?
    • If yes, what are the use cases?
    • If no, we can just keep one and remove the other.
  • Is it really good enough to generate v3 from v2? Need to think about bad cases
    • If good enough, we can still generate v3 from v2
    • If there are bad cases and would cause problems, well, need some new plans

@myers
Copy link
Copy Markdown
Contributor Author

myers commented Mar 29, 2026

For "v3", some questions in my mind:

* Should it co-exist with v2?

It does

  * If yes, what are the use cases?

To allow tools that use openapi 3 specs to generate api clients.

  * If no, we can just keep one and remove the other.

Sounds like a breaking change! ;). The v3 spec is generated from the v2 spec.

* Is it really good enough to generate v3 from v2? Need to think about bad cases

Yes it is good enough, at least for the one library I tried (see PR description). I have tried Fern for python as well, which correctly created the client, but I didn't get a chance to test it yet.

  * If good enough, we can still generate v3 from v2
  * If there are bad cases and would cause problems, well, need some new plans

You can take a peek at it https://github.com/myers/gitea/tree/feat/openapi3-conversion

BTW: I 100% recommend going with spec first API, this was just trying to deliver a feature I needed without rocking the boat too much.

@TheFox0x7
Copy link
Copy Markdown
Contributor

Should it co-exist with v2?

There are still generators that consume v2 only. Though for every language you'll find one that does v3 only so it cuts both ways. I'd drop v2 but we'd have to have native generation first which swagger does not provide

BTW: I 100% recommend going with spec first API, this was just trying to deliver a feature I needed without rocking the boat too much.

I don't see a way for this here. AFAIK you'd have to rework entire api, plus I very much doubt anyone here wants to write yaml/json instead of go. And sure, you could write spec in go, serialize it to yaml, then generate server.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Maybe we can leave the discussion to the future.

By the way, actually our framework has the ability to fully manage the route endpoints, much better than chi (example: Refactor auth middleware #36848)

So maybe now it is feasible to do something like huma (or use huma as the base framework and we call it after we have parsed our routes)

@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 Mar 29, 2026
@wxiaoguang wxiaoguang removed the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Mar 29, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

By the way, I removed the "breaking" label.

I doubt whether there are really many people using "delete reaction endpoint" and would be really affected.

To prevent from frightening most unrelated users, I think this PR doesn't need to be marked as "breaking"

@wxiaoguang wxiaoguang merged commit 2633f96 into go-gitea:main Mar 30, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.26.0 milestone Mar 30, 2026
@GiteaBot GiteaBot added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Mar 30, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 30, 2026
* main: (35 commits)
  Correct swagger annotations for enums, status codes, and notification state (go-gitea#37030)
  Update Nix flake (go-gitea#37024)
  Bump go and python versions in nix flake (go-gitea#37031)
  Make task list checkboxes clickable in the preview tab (go-gitea#37010)
  Add support for in_progress event in workflow_run webhook (go-gitea#36979)
  Fix various problems (go-gitea#37029)
  Update AI Contribution Policy (go-gitea#37022)
  Migrate from webpack to vite (go-gitea#37002)
  Upgrade yaml (go-gitea#37015)
  Fix issue label deletion with Actions tokens (go-gitea#37013)
  Hide delete branch or tag buttons in mirror or archived repositories. (go-gitea#37006)
  Update AGENTS.md with additional guidelines (go-gitea#37018)
  Optimize 'refreshAccesses' to perform update without removing then adding (go-gitea#35702)
  Fix relative-time RangeError (go-gitea#37021)
  Restyle Workflow Graph (go-gitea#36912)
  Update message severity colors, fix navbar double border (go-gitea#37019)
  Clean up checkbox cursor styles (go-gitea#37016)
  add missing cron tasks to example ini (go-gitea#37012)
  Add e2e tests for server push events (go-gitea#36879)
  Update JS dependencies (go-gitea#37001)
  ...
silverwind added a commit to silverwind/gitea that referenced this pull request Mar 30, 2026
* origin/main: (69 commits)
  Correct swagger annotations for enums, status codes, and notification state (go-gitea#37030)
  Update Nix flake (go-gitea#37024)
  Bump go and python versions in nix flake (go-gitea#37031)
  Make task list checkboxes clickable in the preview tab (go-gitea#37010)
  Add support for in_progress event in workflow_run webhook (go-gitea#36979)
  Fix various problems (go-gitea#37029)
  Update AI Contribution Policy (go-gitea#37022)
  Migrate from webpack to vite (go-gitea#37002)
  Upgrade yaml (go-gitea#37015)
  Fix issue label deletion with Actions tokens (go-gitea#37013)
  Hide delete branch or tag buttons in mirror or archived repositories. (go-gitea#37006)
  Update AGENTS.md with additional guidelines (go-gitea#37018)
  Optimize 'refreshAccesses' to perform update without removing then adding (go-gitea#35702)
  Fix relative-time RangeError (go-gitea#37021)
  Restyle Workflow Graph (go-gitea#36912)
  Update message severity colors, fix navbar double border (go-gitea#37019)
  Clean up checkbox cursor styles (go-gitea#37016)
  add missing cron tasks to example ini (go-gitea#37012)
  Add e2e tests for server push events (go-gitea#36879)
  Update JS dependencies (go-gitea#37001)
  ...

# Conflicts:
#	package.json
#	pnpm-lock.yaml
#	tests/e2e/utils.ts
#	web_src/css/themes/theme-gitea-dark.css
#	web_src/css/themes/theme-gitea-light.css
#	web_src/js/bootstrap.ts
#	web_src/js/features/codeeditor.ts
#	web_src/js/modules/errors.test.ts
#	webpack.config.ts
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 pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants