-
Notifications
You must be signed in to change notification settings - Fork 9
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
Consistency in return values #15
Comments
Thanks, Yes, I did see some error message/warning. We have another repository similar to this, We can take action these repos after this. |
The difference between this repository and another repository is that the other one only focuses on the database. So I think it's best to use the final version of the library in the GooglePlayDatabaseMirror repository. |
I'd prefer to finish it here. Using the latest version from the other project would mean starting all over again. Here I'm almost done now – what I described in this issue are the last remaining steps. After reaching consistency, let's increase the version number here and declare it "RC". Next thing needed is what your TODO in the Readme says: "Unit test". In parallel, a set of "calls" can be set up for those methods available in both libraries, and the output compared so you can see what must be adjusted so your cron job won't break. Then the library from here can be taken to a branch of the other project (maybe as git submodule, to not have multiple copies to keep in sync), and adjustments be made there. Funny coincidence: that other project has quite some parallels to what I want to use this library for. There are 14k5 apps in my database currently 😆 With the difference that my apps are from multiple sources (Play, F-Droid, my repo, Aptoide, Xposed), my cron job runs 4 times a day and merges those sources – each time a subset of the database to not hit some wall one day). For 2-3 days now, GooglePlayWebServiceAPI covers the permission part for Play Store. Once that's confirmed (had to fix a minor mapping issue yesterday, seems stable since), other details like ratings, price etc will follow to replace my "old code" grown since 2014. Well, TL;DR: Do I have your "Go" to do the polishing here as lined out above? Maybe your last sentence was just ambiguous – and you did in fact mean using the final version from here, once completed, and bring it "over there"? |
Yes, you can. Thanks for your description.
GooglePlayWebServiceAPI class used at GooglePlayDatabaseMirror repository. |
Done. Also increased version in the file's header (was tempted declaring it "1.0", which it probably should be: according to semantic versioning, this is a release "breaking backward compatibility" (e.g. Please review the PR thoroughly. If you consider it "finalizing", maybe set version to "1.0" – otherwise do not merge but point out what is missing, so that gets addressed first. Thanks! |
Thanks, We still not have a good example file. Maybe need to adding some different example to |
Cool!
I just thought to point to the wiki, which already has several examples and basically covers all use cases (and which I need to update again, now that the PR has been merged). I don't think it's necessary to have even more "examples" in some PHP file. Running a series of different calls isn't very helpful, so one needs to copy/paste anyway. I'd rather have 2 or 3 general examples there (for a quick test to see "ah, it's working!") and point to the wiki for further details. Duplicating everything doesn't ease maintenance – just the opposite. But well, that's my opinion. I won't keep you from creating the "wider variant" 😉 |
Yes. You prepared the wiki very well. |
Thanks! I've just brought the wiki pages up-to-snuff now. You should take a look at them using Github's new dark theme – be prepared to be blown away (before reading 🙊)
Be welcome to copy what you think should be copied 😉 Speaking of copy: with the entire framework of my engine using GPLv2, and this class here almost entirely being rewritten by me now: would you be OK with me declaring it GPLv2 in my framework when I make it public one day? There's no ETA for that (it would need a lot of cleanup before from all the embedded test cases and debug code), so currently the question is rather hypothetical. And should it one day happen, I'll of course point here, credits and all, incl. pointing out the different license. Background: to my knowledge, MIT and GPLv2 have some "compatibility issues". So I just want to make sure not to fall into some trap then. |
Done. cee47f7 This repository is licensed under MIT right now. |
Sure. GPLv2 is a "viral" copy-left license. If someone wants to use the code, the resulting product must use GPLv2 as well and be made public when the product is published – quid pro quo. And other than GPLv3 it does not allow to be monetarized in a service without its stack being again made available openly under GPL (so one can setup the very same service freely). I still consider switching to AGPLv3+ which is similar in that aspect; GPLv3 dropped it. I have no issues with others using my code (else I wouldn't make it open source). But I don't like the idea of others trapping people into walled gardens with my code, making money with it and not sharing that with me, or at least contributing back in any way. MIT would allow that, as would Apache-2.0. Hence I'd choose either GPLv2 or AGPLv3+ (currently, all my code is under GPLv2 – while all my books, articles and other reading material uses CC BY-NC-SA for the very same reasons). If you want details, just use your favorite search engine and look for "google hates agpl" 😆 Examples: Quoting from the former:
And from the latter:
Why? It would force them to open-source that stuff 🤣 And I want more open source, hence I like to spread that "virus". |
@BaseMax is there something missing here, or shall we close this issue? |
It seems this issue is too old for 3 years ago :) Nice man. |
Hehe, then maybe we close it and open a new one should we detect some "missing stuff"? Should I see some while working on something, I'd simply adjust that along the path. |
During my recent implementations and rewrites, I've introduced some "error reporting" to make it easier for the caller to figure if and what might have gone wrong. On error, several methods now return an array like
[success:0, message:"reason"]
But not all of them – for example, most of the search/browse methods supposed to return a simple array of package names don't have this. There would be two options to reach (a sort of) consistency:
[success:0, message:reason]
instead (and if so, includesuccess:1
with a "good result) also for those methods that currently do not, moving the "real results" into a "sub-array" (which then, on error, could be empty or just not present at all)getLastError()
method for obtaining the reason for all search/browse methods while keeping the current behavior for the others.I'd prefer the first approach (so it's completely consistent) with the "empty real result". This combines the best of two worlds, e.g.
I'd even go as far as to always include the
message
key, just leaving its value empty on success. That would make things most consistent.What's your stance? If you agree, I'd go over the entire class another time and make it consistent:
success:0
only on errors – not generally on "no results" for a user-specified search (I vaguely remember I accidentally made it such in one case).message
then holds the reason (e.g. the HTTP response, or parse error when an expected pattern didn't match, etc)success:1
on success.message
then is present but usually empty, but might eg hold a hint on why the result set is empty (like "no hits").If we want to fix it, we want to do that as early as possible – before there are users whose code would otherwise break on some update.
The text was updated successfully, but these errors were encountered: