BaseMax / GooglePlayWebServiceAPI

Tiny script to crawl information of a specific application in the Google play/store base on PHP.
MIT License
37 stars 9 forks source link

Minor improvements #10

Closed IzzySoft closed 3 years ago

IzzySoft commented 3 years ago

Continuing as discussed. For now:

More to come, hence "draft".

IzzySoft commented 3 years ago

Yuck, all that JavaScript overload: Github won't let me set "draft", the JS fails. So please let's stick with the prefix – I'm afraid if you set it to draft I cannot finalize…

BaseMax commented 3 years ago

No worry,

This pull request is still a work in progress Draft pull requests cannot be merged.

BaseMax commented 3 years ago

let's me know when it's ready for Merge. I'm not in a hurry and I do not want you to fall behind in your work.

Thanks.

IzzySoft commented 3 years ago

Ugh – no I'm curious: did you make it draft? Or did the "DRAFT:" prefix do? Or did the button work with a delay? :thinking:

And sure, I'll let you know then.

BaseMax commented 3 years ago

Introducing draft pull requests

https://github.blog/2019-02-14-introducing-draft-pull-requests/

Read more: https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests#draft-pull-requests

Preview button: pullrequest-send

IzzySoft commented 3 years ago

Thanks @BaseMax – if you create a normal pull request, there's also a link to convert it to draft next to "Reviewers", which didn't seem to work for me (no reaction) – so I assumed you might have turned the key. Nevermind, in the end I'll try to remove the "DRAFT:" prefix and see what happens. Somewhere must be a way to remove the draft status.

OK, another commit was added. I've fixed parseCategories() (which was now missing the array) to parsing available categories directly from play, using the page of GMail (which hopefully won't disappear anytime soon), and added PHPDoc to the methods. Your header at the start of the file doesn't really meet PHPDoc specifications (the @keys you used there are not valid). I've left that for now, pending your review. Let me know if I can turn them to "valid keys" as well (will be necessary if you want to use an API generator, which otherwise might complain).

In the meantime, I maybe check for the "summary iterator" (if my time permits).

BaseMax commented 3 years ago

Okay, so send PR and I will merge it. We can try iterator later in next PR. @IzzySoft

BaseMax commented 3 years ago

Thanks, I change status of PR from draft to normal. then merged.

BaseMax commented 3 years ago

Your header at the start of the file doesn't really meet PHPDoc specifications

What's your suggest?

I have follow this structure in hundreds of tanks. But maybe your suggestion is also interesting and will make me follow this in the next repositories too.

Regards,

BaseMax commented 3 years ago

Your comments (PHPDoc) was excellent. I liked. :+1:

BaseMax commented 3 years ago

My review:

Change

    if ( $proto = @file_get_contents('https://play.google.com/_/PlayStoreUi/data/batchexecute?rpcids=xdSrCf&bl=boq_playuiserver_20201201.06_p0&hl='.$lang.'&authuser&soc-app=121&soc-platform=1&soc-device=1&rt=c&f.sid=-8792622157958052111&_reqid=257685', false, $context) ) { // raw proto_buf data
      preg_match("!HTTP/1\.\d\s+(\d{3})\s+(.+)$!i",$http_response_header[0],$match);

to

    if ( $proto = @file_get_contents('https://play.google.com/_/PlayStoreUi/data/batchexecute?rpcids=xdSrCf&bl=boq_playuiserver_20201201.06_p0&hl=' . $lang . '&authuser&soc-app=121&soc-platform=1&soc-device=1&rt=c&f.sid=-8792622157958052111&_reqid=257685', false, $context) ) { // raw proto_buf data
      preg_match("!HTTP/1\.\d\s+(\d{3})\s+(.+)$!i", $http_response_header[0], $match);

So: Change "test".$var."end" to "test" . $var . "end" Change 1,2 to 1, 2

IzzySoft commented 3 years ago

Thanks, I change status of PR from draft to normal. then merged.

Ugh… The idea was that I can tell when I'm ready. Now I again have commits locally I can no longer push to this PR. Can we, with future PRs, organize this? :wink:

Your header at the start of the file doesn't really meet PHPDoc specifications What's your suggest? I have follow this structure in hundreds of tanks.

Which is why I now left this untouched.

But maybe your suggestion is also interesting and will make me follow this in the next repositories too.

Let me see what it would be to meet the specs:

/** Crawl information of a specific application in the Google Play Store
 * @class     GooglePlay
 * @version   0.3
 * @author    Max & Izzy
 * @copyright MIT https://github.com/BaseMax/GooglePlayWebServiceAPI/blob/master/LICENSE
 * @date      2020-10-19, 2020-10-25, 2020-10-29, 2020-10-30, 2020-12-05, 2020-12-06
 * @webpage   repository https://github.com/BaseMax/GooglePlayWebServiceAPI
 **/

Though the @date tag seems to expect a single date. Not sure how useful the list of dates is, and to what; if you wanted, we could turn that into a history:

@log 2020-10-19 initial version
@log 2020-10-25 something has changed
@log 2020-10-29 something was added

etc. Depending on the doc tool to be used, list of available tags differs. I'd need to find the reference for what I use for ages, but here are some example references:

Maybe better switch to something that's still supported, if I only could find it…

My review:

I had just done that (and being on it, also changed existing $var="value" to $var = "value" etc). But it wasn't yet pushed. Will see if it has conflicts after pulling updates.

IzzySoft commented 3 years ago

PS: Here's the list of tags used with phpDocGen:

@attribute [public|protected|private] <type> <name>
@author <name>
@block <unsplittable_text>
@brief <comment>
@bug [fixed] <comment>
@class <name>
@constant <type> <name>
@constructor <name>
@copyright <name>
@date <date>
@deprecated <text>
@extends <class name>
@inherited
@function <name>
@global <name>
@link <name> <comment> (link 2 item/doc)
@log <date> <comment> (ChangeLog)
@method [static] [public|protected|private] <name>
@package <name>
@param [optional] [reference|ref] <type> <name> <comment> (ref: passed by ref)
@private
@protected
@public
@return [reference|ref] [<type>] [<comment>]
@see <comment>
@since <comment>
@static
@todo <comment>
@use <name> (uses global var)
@variable <type> <name>
@verbatim (all current comment will be <pre>)
@version <comment>
@webmodule <html_path>
@webpage [path] <page>
BaseMax commented 3 years ago

Got it. Thanks for prepare this list.

organize this?

Sure, Sorry for this. I understand that others respond late to the Pull Request. I do not want anyone like me to feel this way in the past. I did not mean it (

For future: sure. I will ask you when want to merge something.

IzzySoft commented 3 years ago

Hehe… No finger-pointing, no. Just wanted to make sure our communication didn't fail :laughing:

OK, next PR on its way. Do you want me to update the header as well (as shown above)? If so, shall I convert the "date list" into a log? Though that would require some digging to attach the right comments on what has changed. Further, if one really needed a log, there's the commit history. So we could stick with the @date line as-is – and decide when some DocGen complains.

BaseMax commented 3 years ago

:)

Yes, please change header comment. Your idea is good. If we had to keep just one date, we can keep date created this repository. (Oct 19, But I created this repository even earlier) Thanks

We can add logs, But I not remember date of old logs. Maybe we can get help from this: https://github.com/BaseMax/GooglePlayWebServiceAPI/commits/main

IzzySoft commented 3 years ago

No prob. We can use the @log and limit it to 2 entries: created, and last updated.

Btw: expect some interesting new fields in the next PR :smile: