-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix API status code for hook creation #2814
Conversation
767f5e7
to
e586a7d
Compare
Codecov Report
@@ Coverage Diff @@
## master #2814 +/- ##
==========================================
- Coverage 27.26% 27.24% -0.02%
==========================================
Files 89 89
Lines 17640 17640
==========================================
- Hits 4809 4806 -3
- Misses 12144 12146 +2
- Partials 687 688 +1
Continue to review full report at Codecov.
|
routers/api/v1/utils/hook.go
Outdated
@@ -69,7 +69,7 @@ func AddOrgHook(ctx *context.APIContext, form *api.CreateHookOption) { | |||
org := ctx.Org.Organization | |||
hook, ok := addHook(ctx, form, org.ID, 0) | |||
if ok { | |||
ctx.JSON(200, convert.ToHook(org.HomeLink(), hook)) | |||
ctx.JSON(201, convert.ToHook(org.HomeLink(), hook)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe change to http.StatusCreated
for readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done? I don't see any changes for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, the changes were lost when I rebased. I've readded the lost changes
@ethantkoenig please also regenerate swagger.json file |
@lafriks Done |
71642b6
to
11c8ea9
Compare
Rebased to rerun CI |
@ethantkoenig fmt error |
11c8ea9
to
2719d4e
Compare
Need to verify that Jenkins & Drone plugins are not affected by this |
2719d4e
to
f9df529
Compare
Thanks than LGTM |
LGTM |
Return 201, instead of 200, when creating a hook via the API; this is in compliance with the Github API, and more generally the semantics of HTTP status codes. Affects the following routes:
POST /:owner/:repo/hooks
POST /orgs/:org/hooks