php-shopify icon indicating copy to clipboard operation
php-shopify copied to clipboard

V2 joeyhub

Open joeyhub opened this issue 5 years ago • 3 comments

This is a work in progress and PR is only for draft.

This version does not care for BC, so would have to be version 2.

  • Supports PHP 7.3+ only, at least initially.
  • Support for very recent shopify API versions only.
  • Raises code quality standard.
  • Fixes structural issues.
  • Code clean.
  • Some bug fixes.
  • Pagination support.
  • Possible optimisations.
  • Intends to mostly deal with current open PRs.

joeyhub avatar Mar 06 '20 20:03 joeyhub

Pull request to consider...

This one can be done: https://github.com/phpclassic/php-shopify/pull/130

It's wanted for things such as if the web hook is only stored and then processed later. But the PR is too much of a quick hack so do it nicely. Check everywhere else using globals.

Might be able to integrate this one on "faith / use at own peril": https://github.com/phpclassic/php-shopify/pull/145

This one is weird: https://github.com/phpclassic/php-shopify/pull/114

Bunch of things. Needs some split out, others ignored. Probably pull in new resources but ignore the graphql stuff.

Needs check: https://github.com/phpclassic/php-shopify/pull/102

Need to find out if this one is something missing, a mistake or more than one way to access something causing confusion.

https://github.com/phpclassic/php-shopify/pull/60

I think I remember seeing an angry comment about this in the git history in a legacy code base doing weird things with the params. This one to be cleaned up and applies.

joeyhub avatar Mar 06 '20 22:03 joeyhub

102 looks like a mistake. 114 I think should be a deferred effort to fill in all missing resources.

joeyhub avatar Mar 09 '20 14:03 joeyhub

Key strategies:

  • Focus on support for very contemporary versions. Intended for Semantic versioning.
  • Avoid design patterns, PSRs, etc. Instead focuses on polishing what's already here.
  • Apply consistency and reduce noise.

Key Improvements:

  • This solves a nagging issue of the config being global (multiple shops problem).
  • Minimal extensibility and injection provided to allow users to more easily extend rather than having to edit / fork the library.

Notes:

  • Some messy exception handling to support some legacy app cases.

Current Test Status:

  • Unit Tests work but have limited coverage.
  • Will test it with my app, but coverage is also very minimal (no use of auth helper currently, most entities, graphql, etc).

joeyhub avatar Mar 09 '20 18:03 joeyhub