Skip to content

Fix /orgs/{:orgname}/repos post endpoint #11332

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

Closed
wants to merge 2 commits into from
Closed

Conversation

iondune
Copy link

@iondune iondune commented May 8, 2020

Fixes the issue described in #11331

CreateOrgRepo expects a parameter named :org but the route uses :orgname (see here). Deprecated endpoint uses the same function, but its route uses :org so it works. This changes updates CreateOrgRepo to use the param name from the route, and updates the deprecated route to use the same.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 8, 2020
@zeripath zeripath added this to the 1.12.0 milestone May 8, 2020
@6543
Copy link
Member

6543 commented May 8, 2020

@iondune now it does not match with swagger docu at all :(
can you either update swagger docu or move all to org as param name?

@lunny lunny added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label May 8, 2020
@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 May 9, 2020
@6543
Copy link
Member

6543 commented May 9, 2020

@adelowo @zeripath I know this fix it but the doc is outdated them :/

https://github.com/go-gitea/gitea/pull/11332/files#diff-6c945bcfcee3c77d704a5bd4218c6b68R309

needs an update + make generate-swagger

@zeripath
Copy link
Contributor

zeripath commented May 9, 2020

@6543 that name isn't used by any code and it's not really a problem if the swagger name is different from the actual name used by the code.

(Although I get your point that the swagger documentation is part of the documentation for the code and having it incorrect makes the whole process error prone.)

@6543
Copy link
Member

6543 commented May 9, 2020

ok orgname -> org is not an option if we dont like to refactor all other functions :(

@6543
Copy link
Member

6543 commented May 9, 2020

and org is used on all swagger docs ... -> this need a refactor but I'm ok with this now after having a look at this part of the api .... just meh

PS: the bug was an issue of mine sorry for that - didn't had a look at the path param just used the one swagger had documented

Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

this will confilct with #11339

and I personaly prevere #11339

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

This will break multiple other org endpoints.

The correct solution is to change all of the :orgname endpoints to :org

@lafriks
Copy link
Member

lafriks commented May 11, 2020

Closing as already fixed in other PR

@lafriks lafriks closed this May 11, 2020
@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. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants