Skip to content
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

Ok6245 support api v2.1 #17

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ok6245
Copy link
Contributor

@ok6245 ok6245 commented May 13, 2024

Add support to PHP wrapper for API v2.1.

Major updates for this proposed wrapper Release number 2.1.1:

  • Require PHP version 7.4.0 or higher (current req is 5.6 which is unsupported and prevents using new PHP features)
  • Create new subclass API_v2_1 and add to ApiFactory
  • Refactor the "validFeeds" array with additional data for each feed name to eliminate the duplication of feed names and the need to override BaseApi methods for each new API version. Retroactively applied new format to previous API feed lists. With this refactor, future API should not require any code changes except for creating a new subclass with the validFeeds array in the new format.
  • I created the v2.1 feeds list as best I could from looking at the Online Documentation. Other wrappers were only a little helpful because they appear to differ from each other. I put the v2.1 feeds list in package order (CORE feeds, STATS feeds, etc). This helps with comparing feed lists using a DIFF type tool because looking at the Online Documentation is tedious and error-prone.
  • revert the ellipsis ... token syntax in non-public method parmlists to ordinary array syntax for passing api variables/values. We're currently doing the conversion in 3 places where we could just do it once in BaseApi::getData().
  • update the composer.json and README.md for new PHP version requirement and proposed new wrapper Release number 2.1.1

A housekeeping update:
I'm assuming that the general idea for wrapper Release numbers is to be representative of the highest API version that the wrapper supports.

The current, latest PHP wrapper (and Composer package) release number at this time is 2.1.0. That may have been a poor choice (by me) for that wrapper release number because although it contained many, many updates, it did not specifically address API v2.1. It only supported up to API v2.0.

This pull request contains updates to specifically support up to API V2.1.

To get back to syncing the wrapper Release with the API version it contained, I'm proposing that this Pull Request is to create wrapper Release 2.1.1. Other updates for API v2.1 (if ever) can be given wrapper release numbers 2.1.x.

And when a future API top level version is created, say API v3.0, the wrapper to support that would get Release number 3.0.0.

- change simple array of feeds to array of feed => feed-settings (idea from mysportsfeeds-node wrapper)
- retrofit new array format to all API versions
- update BaseApi methods to process new array format
- subclasses of BaseApi don't need to override __determineUrl() anymore
- updated __determineUrl() avoids duplication of feed names
- feed list for each API version updated as best I can to match online API Documentation
- new module only needs the v2.1 validFeeds array, in new refactored format
- feed list taken from online API Documentation for v2.1 as best I can
…tead of "..." token.

We convert the ellipsis ... token format to key/value array everywhere parameters are needed anyway. Just do it once.
…r msg, not return 404 response.

Standardize verbose output with print() and E-O-L appropriate for running environment cli or web server.
- MySportsFeeds::buildVersion = '2.1.1' to show support for API v2.1
- remove unneeded "use" statements from MySportsFeeds
- update composer.json for required PHP version from 5.6 to 7.4.0
- update README.md with proposed new Release number 2.1.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant