Evaluate possibility to incorporate changes made to forks

[waldyrious]

The changes are relatively straightforward, and some of them may not apply anymore. We should evaluate if that's the case, otherwise we should merge them in.

• Changes from @devlet's fork -- 2 commits:
  • 9bfa241: Switch methods and variables from private to protected so the class be be easily extended
  • 1144b7c: Avoid MD5 errors
• Changes from @nasirkhan's fork -- 2 commits:
  • be582ab: fixed and updated composer.json
  • c664f00: updated cURL path
• Changes from @MhdSyrwan's fork -- 1 commit:
  • ce1eb62: Moved login() out of the constructor

From a cursory look, these changes seem sensible (if up-to-date), except perhaps for the "avoid MD5 errors" one. Calling the authors to comment here :)

[waldyrious]

c664f00 ("updated cURL path") probably is obsolete, given #21 and #25.

[Xymph]

Yes, and be582ab was already incorporated: this and 9de44eb.

Edit: ce1eb62 was already merged back too: 062b3d7. And 9bfa241 as well: eb01046.

I am not sure why the MD5 value should be commented out. Other than that, it would seem the master already contains all relevant changes from the forks.

[waldyrious]

Awesome, thanks for clarifying. Let's wait for @nasirkhan and @MhdSyrwan* to comment -- ideally they would sync up their forks (or delete them if they can now use the upstream directly), so that the network graph is cleaned up.

* @devlet seems to be inactive, so I'm nor sure how realistic it is to expect a reaction from his side.

[devlet]

Thanks for incorporating all these changes :)

I could not make setText() work without commenting the md5 => md5($text) parameter, which I considered ok since it seems redundant as it can be derived directly from the text param.

[Xymph]

Thanks for popping in here. :) md5 is a checksum, so while I wouldn't know why it doesn't work in your case, I prefer to keep it enabled by default.
Since the token parameter is sent (nearly) at the end of the request (as recommended) you should still be guarded against data corruption problems.

[nasirkhan]

thanks for looking into the issues. It is great that the project is active again. 😄

[devlet]

It's fine be me if we just close the md5 issue for now.

[Xymph]

Well, there isn't an open md5 issue in the main project. waldyrious brought up this topic to see if your fork (and nasirkhan's and MhdSyrwan's too) can be deleted or synced up with the master, in order to clean up the network graph. If so, that takes an action on behalf of all three of you.

[devlet]

I tested current master branch with my custom code and it's working without my md5 change. I just synced my copy of upstream's master branch. Is that enough? Or shall I also delete my feature/md5 branch to cleanup de graph?

[waldyrious]

@devlet if you don't mind, please delete the branches, yes (feature/md5 and feature/protected), as they still appear in the network graph.

[devlet]

I deleted both branches :)