Added read-only attachments to the Releases API #63

stefan-lacatus · 2017-06-30T07:35:32Z

Added a assets array to the releases GET request. This is a required change for go-gitea/gitea#2084

Get releases GET /repos/:owner/:repo/releases[/:id] - now includes assets
List assets: GET /repos/:owner/:repo/releases/:id/assets
Get single asset: GET /repos/:owner/:repo/releases/assets/:id

List assets: GET /repos/:owner/:repo/releases/:id/assets Get single asset: GET /repos/:owner/:repo/releases/assets/:id

lafriks · 2017-06-30T07:43:03Z

gitea/attachment.go

+import "time"
+
+// a generic attachment
+type Attachment struct {


At least in SDK I think we should call it also as Asset so that there would not be different names for same thing.
Also type comment must start with type name like // Asset represents a release attachments

But because the Attachments can be used in multiple places (releases, issues, comments) is it worth to make this change? Or should we rename Attachements to assets everywhere? Having two words for the same thing is confusing.

Yes but attachments for issues and comments most probably wont have same structure anyway (in API).

lafriks · 2017-06-30T07:46:30Z

gitea/release.go

+}
+
+// GetReleaseAsset gets all the assets of a release in a repository
+func (c *Client) GetReleaseAsset(user, repo string, releaseId int64, assetId int64) (*Release, error) {


releaseId must be called releaseID, assetId -> assetID

Run make lint to check if your golang code validates against GoLang naming convention

…s the latest published full release for the repository. Draft releases and prereleases are not returned by this endpoint. Signed-off-by: Petrisor Lacatus <[email protected]>

Signed-off-by: Petrisor Lacatus <[email protected]>

bkcsoft

#namingthings

bkcsoft · 2017-07-01T18:26:59Z

gitea/release.go

@@ -46,6 +47,33 @@ func (c *Client) GetRelease(user, repo string, id int64) (*Release, error) {
 	return r, err
 }

+// ListReleaseAssets gets all the assets of a release in a repository
+func (c *Client) ListReleaseAssets(user, repo string, id int64) (*Release, error) {


These should be be named Attachments instead of Assets since Gitea calls them that :)

@bkcsoft but in GitHub api they are called assets and we want API to be compatible with GitHub

Indeed, but the function should still be called Attachment, even though the payload has Assets :)

Ok, so everywhere in the code we still refer to them as Attachments, and the only place where "asset" appears is in the json for the release?

@bkcsoft I think it will be confusing that endpoints and in json it is called assets but in sdk attachments

So, how should I name things? A decision has to be reached by you guys...

@stefan-lacatus after talking to @bkcsoft we agreed on naming that in golang code we should use name Attachment and in json payloads and url's use GitHub name asset

The only place we refer to attachments as assets is in the api json response Signed-off-by: Petrisor Lacatus <[email protected]>

stefan-lacatus · 2017-07-03T12:54:47Z

I've changed the naming as suggested. Now we refer to release attachments as Attachments everywhere in the code, except in the JSON response where they are called assets and in the endpoint names

bkcsoft · 2017-07-03T18:20:57Z

gitea/release.go

@@ -46,6 +47,33 @@ func (c *Client) GetRelease(user, repo string, id int64) (*Release, error) {
 	return r, err
 }

+// ListReleaseAttachments gets all the assets of a release in a repository
+func (c *Client) ListReleaseAttachments(user, repo string, id int64) (*Release, error) {


This should return a []*Attachment, not *Release https://developer.github.com/v3/repos/releases/#list-assets-for-a-release

bkcsoft · 2017-07-03T18:21:51Z

gitea/release.go

+}
+
+// GetReleaseAttachment gets all the assets of a release in a repository
+func (c *Client) GetReleaseAttachment(user, repo string, releaseID int64, attachmentID int64) (*Release, error) {


Same here, []*Attachment

Here it should have been a *Attachment since we are only returning one instance. I've also updated the function description as it was incorrect

Updated method description Signed-off-by: Petrisor Lacatus <[email protected]>

bkcsoft · 2017-07-04T17:32:24Z

@lafriks mind reviewing again? 🙂

lafriks · 2017-07-08T06:40:46Z

LGTM

bkcsoft · 2017-07-09T05:43:59Z

LGTM as well :)

lafriks · 2017-07-12T22:03:23Z

I think we should wait for 1.2.0 to be released before we merge this

bkcsoft · 2017-08-23T21:37:17Z

stable/v1.2 has been branched so I'm merging this 🙂

jonasfranz · 2018-03-05T18:54:09Z

I think that this PR could be closed becuase the #90 already implemented this feature.

stefan-lacatus added 2 commits June 30, 2017 09:06

Added assets (attachments) to the releases.

6f90d1d

Added two new endpoints to the release api for attachments.

809b6c0

List assets: GET /repos/:owner/:repo/releases/:id/assets Get single asset: GET /repos/:owner/:repo/releases/assets/:id

lafriks reviewed Jun 30, 2017

View reviewed changes

lafriks suggested changes Jun 30, 2017

View reviewed changes

Fix linting errors

549beb7

stefan-lacatus mentioned this pull request Jun 30, 2017

Added attachments to the releases API go-gitea/gitea#2084

Closed

lafriks added the kind/feature label Jun 30, 2017

stefan-lacatus added 2 commits June 30, 2017 16:38

Added the endpoint GET /repos/:owner/:repo/releases/latest that get…

7f04eaa

…s the latest published full release for the repository. Draft releases and prereleases are not returned by this endpoint. Signed-off-by: Petrisor Lacatus <[email protected]>

Fixed small issues

c6ac402

Signed-off-by: Petrisor Lacatus <[email protected]>

bkcsoft suggested changes Jul 1, 2017

View reviewed changes

Renamed methods having to do with Assets to Attachments.

b8bab7d

The only place we refer to attachments as assets is in the api json response Signed-off-by: Petrisor Lacatus <[email protected]>

tboerger added the lgtm/need 2 label Jul 3, 2017

bkcsoft suggested changes Jul 3, 2017

View reviewed changes

Fixed methods having the wrong return type

dd3b331

Updated method description Signed-off-by: Petrisor Lacatus <[email protected]>

bkcsoft approved these changes Jul 4, 2017

View reviewed changes

lafriks approved these changes Jul 8, 2017

View reviewed changes

tboerger added lgtm/need 1 and removed lgtm/need 2 labels Jul 8, 2017

tboerger added lgtm/done and removed lgtm/need 1 labels Jul 9, 2017

bkcsoft merged commit 11e0aa8 into go-gitea:master Aug 23, 2017

stefan-lacatus mentioned this pull request Dec 3, 2017

Added attachment releases data #83

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added read-only attachments to the Releases API #63

Added read-only attachments to the Releases API #63

stefan-lacatus commented Jun 30, 2017

lafriks Jun 30, 2017

stefan-lacatus Jun 30, 2017

lafriks Jun 30, 2017 •

edited

Loading

lafriks Jun 30, 2017

bkcsoft left a comment

bkcsoft Jul 1, 2017

lafriks Jul 1, 2017

bkcsoft Jul 2, 2017

stefan-lacatus Jul 2, 2017

lafriks Jul 2, 2017

stefan-lacatus Jul 2, 2017

lafriks Jul 2, 2017

stefan-lacatus commented Jul 3, 2017 •

edited

Loading

bkcsoft Jul 3, 2017

bkcsoft Jul 3, 2017

stefan-lacatus Jul 4, 2017 •

edited

Loading

bkcsoft commented Jul 4, 2017

lafriks commented Jul 8, 2017

bkcsoft commented Jul 9, 2017

lafriks commented Jul 12, 2017

bkcsoft commented Aug 23, 2017

jonasfranz commented Mar 5, 2018

Added read-only attachments to the Releases API #63

Added read-only attachments to the Releases API #63

Conversation

stefan-lacatus commented Jun 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lafriks Jun 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkcsoft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefan-lacatus commented Jul 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefan-lacatus Jul 4, 2017 • edited Loading

Choose a reason for hiding this comment

bkcsoft commented Jul 4, 2017

lafriks commented Jul 8, 2017

bkcsoft commented Jul 9, 2017

lafriks commented Jul 12, 2017

bkcsoft commented Aug 23, 2017

jonasfranz commented Mar 5, 2018

lafriks Jun 30, 2017 •

edited

Loading

stefan-lacatus commented Jul 3, 2017 •

edited

Loading

stefan-lacatus Jul 4, 2017 •

edited

Loading