-
Notifications
You must be signed in to change notification settings - Fork 51
Add Attachment API #90
Conversation
Signed-off-by: Jonas Franz <[email protected]>
Signed-off-by: Jonas Franz <[email protected]>
Signed-off-by: Jonas Franz <[email protected]>
Renaming Assets to Attachments Signed-off-by: Jonas Franz <[email protected]>
Signed-off-by: Jonas Franz <[email protected]>
|
LGTM |
Add CreateReleaseAttachment function Signed-off-by: Jonas Franz <[email protected]>
Add DeleteReleaseAttachment Signed-off-by: Jonas Franz <[email protected]>
|
@lunny Could you please re-review this PR because I've added some other functions which includes a file upload. |
Signed-off-by: Jonas Franz <[email protected]>
gitea/attachment.go
Outdated
| } | ||
|
|
||
| // CreateReleaseAttachment creates an attachment for the given release | ||
| func (c *Client) CreateReleaseAttachment(user, repo string, release int64, file *os.File) (*Attachment, error) { |
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.
Why not io.Reader for file content?
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.
Because it is required to get the filename (line 65).
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.
Pass the filename along? (fi.Name(), file)
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"` |
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.
Should I use here attachments or assets (in JSON)?
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.
ref https://developer.github.com/v3/repos/releases/#list-releases-for-a-repository, it should be assets
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.
@JonasFranzDEV this is not resolved.
gitea/attachment.go
Outdated
| 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) |
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.
So that, fileContents is unnecessary. You can just _, err = io.Copy(writer, file)
Signed-off-by: Jonas Franz <[email protected]>
gitea/attachment.go
Outdated
| return nil, err | ||
| } | ||
| part.Write(fileContents) | ||
| io.Copy(part, file) |
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.
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"` |
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.
@JonasFranzDEV this is not resolved.
Signed-off-by: Jonas Franz <[email protected]>
Depends on and blocks go-gitea/gitea#3478
Adds
Attachmentsfield toReleaseand improves theAttachmentmodel by making it swagger compatible.TODO