Skip to content

Conversation

@6543
Copy link
Member

@6543 6543 commented Jun 11, 2020

as title

come up in #11758 (comment)

@6543 6543 changed the title API: Move Setting function into its own package WIP: API: Move Setting function into its own package Jun 11, 2020
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 11, 2020
@6543
Copy link
Member Author

6543 commented Jun 11, 2020

@mrsdizzie ping

WIP because if there are more usefull settings to expose in the UI scope:

-> change GetAllowedReactions to a getGeneralUISettings ...

@mrsdizzie
Copy link
Member

@6543 that change is fine but I mean that you shouldn't have an endpoint for one single setting like /settings/allowed_reactions -- it should be /settings/ui that can return

{
  "allowedReactions": []
}

That way you can eventually add other settings without having to create new endpoints for each one, just like how the check for mirrors uses /settings/repository.

I only pick 'ui' because that is the section in config.ini just as repository is for the other example

@6543 6543 changed the title WIP: API: Move Setting function into its own package API: Move Setting function into its own package Jun 12, 2020
@6543 6543 changed the title API: Move Setting function into its own package API: Move AllowedReactions endpoint into GetGenneralUI endpoint + creat new swagger section settings Jun 12, 2020
@6543
Copy link
Member Author

6543 commented Jun 12, 2020

@mrsdizzie done

@mrsdizzie
Copy link
Member

Test fail changes seem good otherwise

@lafriks lafriks added the modifies/api This PR adds API routes or modifies them label Jun 12, 2020
@lafriks lafriks added this to the 1.13.0 milestone Jun 12, 2020
@6543
Copy link
Member Author

6543 commented Jun 12, 2020

@mrsdizzie passed - fail seems unrelated

Copy link
Member

@mrsdizzie mrsdizzie left a comment

Choose a reason for hiding this comment

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

🚀

@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 Jun 12, 2020
@GiteaBot GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jun 22, 2020
@GiteaBot GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Jun 22, 2020
@6543
Copy link
Member Author

6543 commented Jun 22, 2020

🚀

@techknowlogick techknowlogick merged commit fc2f2c7 into go-gitea:master Jun 22, 2020
@6543 6543 deleted the api_generalize-UI-setting branch June 22, 2020 18:29
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
…at new swagger section settings (go-gitea#11854)

* move Setting function into its own package

* swagger add&use new section "settings"

* move api AllowedReactions into general UI-Settings endpoint

* prepare TEST

* lint
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants