Skip to content

Conversation

@rickhewes
Copy link

Add the ability to add optional metadata.

Add the ability to add optional metadata.
@factyy
Copy link
Owner

factyy commented Mar 9, 2022

@rickhewes and just a small hint: don't use ambiguous commit messages, if you fix a typo just write it in the commit message :)

@factyy
Copy link
Owner

factyy commented Mar 9, 2022

Everything else looks good, I will merge your PR this week if you fix the issues

@rickhewes
Copy link
Author

I changed the commit message. What else would you like changing? I don't see your comments anywhere....

raise 'New file uploading is not supported!'
end

if metadata.nil? then
Copy link
Owner

Choose a reason for hiding this comment

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

Don't use trailing then unless you are writing a one-liner.
The most common way to write such statements is metadata ||= {}

metadata[:filename] = file_name

pairs = []
metadata.each do |key, value|
Copy link
Owner

Choose a reason for hiding this comment

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

It is easier to use a brace-delimited block and place this code right at the request construction. Thus you will not need pairs (which is also a bad name choice too)

@factyy
Copy link
Owner

factyy commented Mar 10, 2022

Sorry, forgot to publish the review :)

@factyy
Copy link
Owner

factyy commented Mar 11, 2022

@rickhewes , looks nice! I will merge your PR and release a new version in a few days

@rickhewes
Copy link
Author

@factyy I have another addition re CHUNK_SIZE. Maybe hold off releasing another version. Let me get the pull request going.

@factyy
Copy link
Owner

factyy commented Mar 14, 2022

Ok, lets do it this way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants