-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Added attachments to releases API #3075
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
Conversation
CI failed |
Yes, but i don't understand why... I've already run go fmt and the result was the last commit added c788b3d |
routers/api/v1/api.go
Outdated
@@ -461,6 +461,16 @@ func RegisterRoutes(m *macaron.Macaron) { | |||
m.Combo("/:id").Get(repo.GetRelease). | |||
Patch(reqToken(), reqRepoWriter(), bind(api.EditReleaseOption{}), repo.EditRelease). | |||
Delete(reqToken(), reqRepoWriter(), repo.DeleteRelease) | |||
m.Combo("/latest").Get(repo.GetLatestRelease) | |||
m.Group("/:id", func() { | |||
m.Combo("").Get(repo.GetRelease). |
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.
Lines 466-468 duplicate lines 461-463; please remove one or the other.
routers/api/v1/repo/release.go
Outdated
@@ -94,6 +144,29 @@ func ListReleases(ctx *context.APIContext) { | |||
ctx.JSON(200, rels) | |||
} | |||
|
|||
// GetLatestRelease Gets the latest release in a repository. Draft releases and prereleases are not returned |
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.
nit: Gets -> gets
@@ -94,6 +102,10 @@ func (r *Release) TarURL() string { | |||
|
|||
// APIFormat convert a Release to api.Release | |||
func (r *Release) APIFormat() *api.Release { | |||
apiAttachments := make([]*api.Attachment, len(r.Attachments)) |
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.
This will only work if r.Attachments
has been populated before calling APIFormat()
. This means we will need to update each endpoint that uses Release.APIFormat()
accordingly.
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.
I am populating the list in loadAttributes on line 71->78. Is that ok?
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, I missed that. Never mind then
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.
One more
routers/api/v1/api.go
Outdated
Delete(repo.DeleteRelease) | ||
m.Group("/assets", func() { | ||
m.Combo("").Get(repo.ListReleaseAttachments) | ||
m.Combo("/:assetId").Get(repo.GetReleaseAttachment) |
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.
This route is incorrect; see https://developer.github.com/v3/repos/releases/#get-a-single-release-asset
Fixed small typo
Quick question: I see you are declaring some swagger definitions as code comments. Is there any documentation on the syntax? Should I do it as well? |
@stefan-lacatus Yes, please do. Here are the relevant docs; let me know if you have any questions. |
Attachments are in misc because they could be reused for issues and comments
Swagger definitions are also included. I think we should decide if we keep the endpoint which gets a single asset of a single repository. See the discussion on go-gitea/go-sdk#83 (comment) |
@stefan-lacatus any progress? It would be awesome to see this feature soon 😄 |
This feature got implemented in #3478 in the meantime. So I think that this PR should be closed. |
Closed by #3478 |
See #2084 for details. This is exactly the same PR, just on top of master.
Needs go-gitea/go-sdk#83 to work.