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

V2 joeyhub #148

Draft
wants to merge 31 commits into
base: v2.0
Choose a base branch
from
Draft

V2 joeyhub #148

wants to merge 31 commits into from

Conversation

joeyhub
Copy link

@joeyhub joeyhub commented Mar 6, 2020

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
Copy link
Author

joeyhub commented Mar 6, 2020

Pull request to consider...

This one can be done: #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": #145

This one is weird: #114

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

Needs check: #102

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

#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
Copy link
Author

joeyhub commented Mar 9, 2020

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

@joeyhub
Copy link
Author

joeyhub commented Mar 9, 2020

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).

@tareqtms tareqtms changed the base branch from master to v2.0 April 25, 2020 15:10
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.

None yet

1 participant