basvandorst / StravaPHP

Strava API REST client with OAuth authentication
MIT License
184 stars 67 forks source link

Library Compatibility #77

Closed MGatner closed 2 years ago

MGatner commented 2 years ago

Hi maintainers! I'm working with a client who needs Strava integrations - excited to see that someone has already wrapped this up into an SDK. There are a few incompatibilities (Guzzle being a big one, I see there has been some work on that). I will send a few PRs over to get this up-to-speed, then maybe some recommendations for a next major release that would have a lot of QOL improvements for PHP >= 7.4.

Please let me know if there are any guiding principles I should be aware of, as you have no contributing docs. Happy to hop on Discord/Slack/Discussions whatever.

MGatner commented 2 years ago

Starting with static analysis, #76

MGatner commented 2 years ago

Updating unit tests, #78

MGatner commented 2 years ago

Guzzle 7 compatibility, #79

MGatner commented 2 years ago

@djjavo @vredeling Since we're almost over the basics of compatibility... what are your thoughts on next steps for this library? A couple of ideas I would be interested in:

  1. Drop unsupported versions of PHP (< 7.4). This would allow the next major version to add a bunch of new PHP features (most notably: property types)
  2. Implement entities. Instead of returning everything as an array of data, make it into an actual PHP class with expected types. I haven't worked with the API yet so maybe things aren't as consistent as I'm expecting, but it would be a big boost for working with the library.
MGatner commented 2 years ago

One last addendum while I'm here... I'm not a total Git Flow fan but I do like some of their structure - the biggest one being using a develop branch for your primary development and retaining master for releases. If you're amenable I would love to work with you on that.

vredeling commented 2 years ago

@djjavo @vredeling Since we're almost over the basics of compatibility... what are your thoughts on next steps for this library? A couple of ideas I would be interested in:

  1. Drop unsupported versions of PHP (< 7.4). This would allow the next major version to add a bunch of new PHP features (most notably: property types)
  2. Implement entities. Instead of returning everything as an array of data, make it into an actual PHP class with expected types. I haven't worked with the API yet so maybe things aren't as consistent as I'm expecting, but it would be a big boost for working with the library.

I'm not ready to drop php7 support just yet on the existing branches, as I don't know what systems the current users of this lib are working on. But I'm quite kind to make the jump to 8 myself.

Entities, would be useful as long as they exactly mimic the models in the current API. See: https://developers.strava.com/docs/reference/#api-models

With regards to git flow, I'm totally on board with using that. And for its shortcomings, at least it is a well known standard.

MGatner commented 2 years ago

PHP 7.4 has security support through December. I'm proposing ^7.4 || ^8.0

MGatner commented 2 years ago

With regards to git flow, I'm totally on board with using that

Great! I see there is an old develop branch already - that will need to be brought up to speed:

git switch master
git pull
git sw develop
git reset --hard master
git push -f

Then from the repo settings you can change develop to be the default branch:

Screen Shot 2022-04-21 at 9 14 00 AM
vredeling commented 2 years ago

Hey guess what... I don't have sufficient permissions to change some essentials like the default branch. Contacting @basvandorst about that.

MGatner commented 2 years ago

@vredeling when do you think we could get a new release of the work so far? The Guzzle 7 piece is still blocking most current PHP version.

vredeling commented 2 years ago

@MGatner https://github.com/basvandorst/StravaPHP/releases/tag/2.0.0

If any older version requires a support branch we can do so manually. I'm not expecting many requests to that end, honestly.