Skip to content
This repository was archived by the owner on Jun 8, 2019. It is now read-only.

Add Attachment API #90

Merged
merged 13 commits into from
Feb 28, 2018
Merged

Add Attachment API #90

merged 13 commits into from
Feb 28, 2018

Conversation

jonasfranz
Copy link
Member

@jonasfranz jonasfranz commented Feb 8, 2018

Depends on and blocks go-gitea/gitea#3478

Adds Attachments field to Release and improves the Attachment model by making it swagger compatible.

TODO

  • Add client functions for getting attachments

Signed-off-by: Jonas Franz <[email protected]>
Renaming Assets to Attachments

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz jonasfranz mentioned this pull request Feb 8, 2018
1 task
@lunny
Copy link
Member

lunny commented Feb 9, 2018

LGTM

Add CreateReleaseAttachment function

Signed-off-by: Jonas Franz <[email protected]>
Add DeleteReleaseAttachment

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Copy link
Member Author

@lunny Could you please re-review this PR because I've added some other functions which includes a file upload.

}

// CreateReleaseAttachment creates an attachment for the given release
func (c *Client) CreateReleaseAttachment(user, repo string, release int64, file *os.File) (*Attachment, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not io.Reader for file content?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it is required to get the filename (line 65).

Copy link
Member

@bkcsoft bkcsoft Feb 10, 2018

Choose a reason for hiding this comment

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

Pass the filename along? (fi.Name(), file)

gitea/release.go Outdated
Publisher *User `json:"author"`
PublishedAt time.Time `json:"published_at"`
Publisher *User `json:"author"`
Attachments []*Attachment `json:"attachments"`
Copy link
Member Author

Choose a reason for hiding this comment

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

Should I use here attachments or assets (in JSON)?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@JonasFranzDEV this is not resolved.

@@ -46,23 +46,17 @@ func (c *Client) GetReleaseAttachment(user, repo string, release int64, id int64
}

// CreateReleaseAttachment creates an attachment for the given release
func (c *Client) CreateReleaseAttachment(user, repo string, release int64, file *os.File) (*Attachment, error) {
func (c *Client) CreateReleaseAttachment(user, repo string, release int64, file *io.Reader, filename string) (*Attachment, error) {
// Read file to upload
fileContents, err := ioutil.ReadAll(file)
Copy link
Member

Choose a reason for hiding this comment

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

So that, fileContents is unnecessary. You can just _, err = io.Copy(writer, file)

// Write file to body
body := new(bytes.Buffer)
writer := multipart.NewWriter(body)
part, err := writer.CreateFormFile("attachment", filename)
if err != nil {
return nil, err
}
part.Write(fileContents)
io.Copy(part, file)
Copy link
Member

Choose a reason for hiding this comment

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

The return error should be handled. Sorry, I haven't make it clear on my previous review.

Signed-off-by: Jonas Franz <[email protected]>
gitea/release.go Outdated
Publisher *User `json:"author"`
PublishedAt time.Time `json:"published_at"`
Publisher *User `json:"author"`
Attachments []*Attachment `json:"attachments"`
Copy link
Member

Choose a reason for hiding this comment

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

@JonasFranzDEV this is not resolved.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants