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

Various improvements #16

Merged
merged 16 commits into from
Nov 14, 2017
Merged

Various improvements #16

merged 16 commits into from
Nov 14, 2017

Conversation

simonhamp
Copy link
Collaborator

Hi Will, as you can probably tell from this PR I've been going around with this package for a bit. I'd really like to just push forward with one package rather than publicise my fork.

I appreciate this may seem like a daunting PR to review. Happy to work through it with you, but the main changes are:

  • Simplified the composer dependencies to those that are actually in use. This will make setting up dev environment speedier and lighter
  • Gotten rid of some files that seemed a bit superfluous and generally tidied the package up, renaming some of the main classes to help better identify their purpose
  • Attempted to make the core console command more flexible and readable - it's now possible to use ZeroMQ with any server type (not just WAMP)
  • Simplified the readme to make getting started simpler for newbies
  • Requiring PHP 7.1+

You'll probably notice references to laravel-zmq - please ignore all of this, I've since removed it.

Final note is that this all is most definitely a breaking change that will stop backwards compatibility. So I recommend that you tag a new major release if you merge these changes.

Any issues or questions, let me know and I will try to resolve.

It's more modular now and has a slightly easier to follow flow. `$serverInstance` is mutable and changes throughout the bootup sequence as more parts are added. $ratchetServer is always an instance of the class used at the outset.
Let's start pushing people forwards
As this package relies on my fork of that package, it's better to bundle it and expose via this package.
Remove pelim/laravel-zmq again. Cut back all of Laravel to just what we actually use in *this* package that isn't provided elsewhere
Copy link
Contributor

@gcphost gcphost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@gcphost gcphost merged commit 94efe90 into Askedio:master Nov 14, 2017
@simonhamp simonhamp deleted the patch-laravel-5.5 branch November 14, 2017 17:42
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.

2 participants