search

Wizcorp / node-iap

In-app purchase validation for Apple, Google, Amazon, Roku
262 stars 92 forks source link

google: use orderId for transactionId #56

Closed adrian-gierakowski closed 5 years ago

adrian-gierakowski commented 6 years ago

as discussed in #44

ronkorving commented 6 years ago

LGTM, but would like to hold off on merging, until non-breaking-changes have been merged and released first.

adrian-gierakowski commented 6 years ago

I am a bit uncomfortable with this change as we are changing a meaning of a property in a way that could easily go unnoticed (at least for a while) by unsuspecting users. Maybe the transactionId prop should be deprecated and new name introduced? The only problem being that transactionId seems the best fit for the job, and it would be a pity having to choose a less appropriate one.

ronkorving commented 6 years ago

Thoughts anyone? :)

superandrew213 commented 6 years ago

If we follow semver, and since it will be a breaking change, I think it should be fine. We can also add a changelog.

ronkorving commented 6 years ago

The point of transactionId is to be the unique identifier. So introducing a new property is not a solution for any anxiety caused by this change. I think the right thing to do is indeed to just be very clear in our communication about it. That includes how we number the version and the changelog.

nyaaao commented 5 years ago

@ronkorving @adrian-gierakowski is there any progress on that?

adrian-gierakowski commented 5 years ago

@nyaaao I believe this is ready to merge

nyaaao commented 5 years ago

@adrian-gierakowski do you have any ETA on when a new version will be released?

adrian-gierakowski commented 5 years ago

@nyaaao I don't, as I am not a maintainer for this project

ronkorving commented 5 years ago

Sorry for the delay everyone, I'll get on it!

ronkorving commented 5 years ago

Released as 1.0.0! Thanks for your patience, everyone. I also released 0.10.0 before that btw, with the latest changes that were not yet on NPM.