-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Handle encoding value of "none" #2924
Conversation
If I understand the docs correctly, Content should always be nil when encoding is "none". I think it would be better to continue returning an error on "none", but it could be a better error that explains that this is because the file size is over 1MB and suggests using DownloadContents instead. |
Ah! It seems in fact I have misunderstood! Definitely would be nice to have a better error though. I'll update my PR accordingly. |
Updated to give a nicer error message in that case! |
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.
LGTM. Just a small nit about the test case, but you don't necessarily need to take my suggestion there.
To set expectations, the primary maintainer is out until 9/18, so this PR is not likely to be merged before then.
Codecov Report
@@ Coverage Diff @@
## master #2924 +/- ##
=======================================
Coverage 98.17% 98.17%
=======================================
Files 143 143
Lines 12609 12611 +2
=======================================
+ Hits 12379 12381 +2
Misses 156 156
Partials 74 74
|
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.
Thank you, @GusPrice and @WillAbides !
LGTM.
Merging.
When a file is over 1 MB in a repository, GitHub responds with "none" as the encoding value. Handle this accordingly.