Skip to content

Change lastBytePos to long in MediaHttpDownloader.setContentRange - #985

Merged
mattwhisenhunt merged 1 commit into
googleapis:devfrom
mziccard:int-to-long-set-range
Mar 11, 2018
Merged

mattwhisenhunt merged 1 commit into
googleapis:devfrom
mziccard:int-to-long-set-range

Conversation

@mziccard

Copy link
Copy Markdown
Contributor

This PR tries to fix #937.

This PR breaks clirr check with the following error:

Parameter 2 of 'public com.google.api.client.googleapis.media.MediaHttpDownloader setContentRange(long, int)' has changed its type to long

I don't see any major issue with this as MediaHttpDownloader.lastBytePos field is already a long, but correct me if I'm wrong.

@ahmadyousaf

Copy link
Copy Markdown

As if this functions receives an int, assigning it to long would still have int data. By definition, in java int can hold maximum of 2^31 - 1
If we have a file just larger than 2gb. we cannot specify lastBytePos if it's at the end of the file. On the same note, if you could also update the javadocs for this function as it says: "Sets the content range of the next download request. Eg: bytes=firstBytePos-lastBytePos."

It should rather be "... Eg: bytes=lastBytePos-firstBytePos."

@jamie-everlaw

Copy link
Copy Markdown

Any reason this hasn't been merged?

@mattwhisenhunt mattwhisenhunt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mattwhisenhunt
mattwhisenhunt merged commit 3196df8 into googleapis:dev Mar 11, 2018
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.

5 participants