Skip to content
This repository has been archived by the owner on Aug 25, 2023. It is now read-only.

Media id has changed from string to int64 #295

Merged
merged 1 commit into from
Jan 28, 2020

Conversation

olimpias
Copy link
Contributor

Media struct`s ID type should be int64

@olimpias olimpias requested a review from ahmdrz January 25, 2020 11:58
Copy link
Owner

@ahmdrz ahmdrz left a comment

Choose a reason for hiding this comment

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

Thank you,
why you didn't use json.Number?

@olimpias
Copy link
Contributor Author

olimpias commented Jan 26, 2020

Since the media ID is represented with long value number. I thought int64 is much better and immediately in use as a variable. If you use json.Number, you have to call a method to int64 to use it. If a person wants to use ID as a string, they can convert by themself.

@ahmdrz
Copy link
Owner

ahmdrz commented Jan 26, 2020

There are a lot of problems with these IDs. Instagram changes them quickly. So we cannot be sure about what type are they. In v3 which I'm working on, I try to use json.Number instead of int64 and string.
In this case, does any other problem appears if we use string instead of int64? Are there any other functions that use int64 and we have to change them?

@olimpias
Copy link
Contributor Author

olimpias commented Jan 26, 2020

Currently, JSON response is not mapping with string in other words, media ids are not coming with double-quotes. So it is causing failure for JSON unmarshalling. That is why I created MR that converts to int64. I didn't know that they are frequently changing it. But right now, it is broken. So far, I only faced problem with Media ID. Others are kinda fine.

@ahmdrz ahmdrz merged commit f47fe60 into ahmdrz:master Jan 28, 2020
@olimpias olimpias deleted the media-id-fix branch January 29, 2020 19:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants