Skip to content
This repository has been archived by the owner on Jan 17, 2022. It is now read-only.

refactor(di): Simplify registering configurators #14

Merged
merged 1 commit into from
Oct 15, 2018
Merged

Conversation

k911
Copy link
Owner

@k911 k911 commented Oct 15, 2018

Creating matrix of configuration and http server factory services when some services needs to be
only in specific commands can become quite annoying

BREAKING CHANGE:

  • Server\HttpServerFactory should not be instantiated anymore, due to
    removed hard coupling with ConfiguratorInterface, and make() method
    becomig static. Now use directly: HttpServerFactory::make()
  • Configuring server (using object implementing ConfiguratorInterface)
    now happens in execute method of AbstractServerStartCommand
  • Server\Configurator\ChainConfigurator now accepts
    ConfiguratorInterface variadic starting from second argument and
    implements IteratorAggregate retruning its configurators to ease DI usage (see
    src/Bridge/Symfony/Bundle/Resources/commands.yaml)

Creating matrix of configuration and http server factory services when some services needs to be
only in specific commands can become quite annoying

BREAKING CHANGE:

- Server\HttpServerFactory should not be instantiated anymore, due to
removed hard coupling with ConfiguratorInterface, and `make()` method
becomig static. Now use directly: `HttpServerFactory::make()`
- Configuring server (using object implementing ConfiguratorInterface)
now happens in execute method of AbstractServerStartCommand
- Server\Configurator\ChainConfigurator now accepts
ConfiguratorInterface variadic starting from second argument and
implements IteratorAggregate retruning its configurators to ease DI usage (see
src/Bridge/Symfony/Bundle/Resources/commands.yaml)
@k911 k911 self-assigned this Oct 15, 2018
@@ -100,10 +101,12 @@ protected function configure(): void
exit(1);
}

$this->server->attach($this->serverFactory->make(
$server = HttpServerFactory::make(
Copy link

Choose a reason for hiding this comment

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

Rename "$server" which has the same name as the field declared at line 28.

@codeclimate
Copy link

codeclimate bot commented Oct 15, 2018

Code Climate has analyzed commit 34c99e0 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Clarity 1

The test coverage on the diff in this pull request is 75.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 41.2% (0.6% change).

View more on Code Climate.

@k911 k911 merged commit a34d59c into develop Oct 15, 2018
@k911 k911 deleted the refactor/di branch October 15, 2018 19:33
k911 pushed a commit that referenced this pull request Oct 20, 2018
##  (2018-10-20)

* docs(quick-start): Improve documentation ([2441afb](2441afb))
* refactor(boot-manager): Use tagged services injection ([0b1499b](0b1499b))
* refactor(configurator): Create ChainConfigurator ([6c2aafd](6c2aafd))
* refactor(configurator): Create GeneratedCollection and use in chains ([bbb5034](bbb5034))
* refactor(di): Simplify registering configurators (#14) ([a34d59c](a34d59c)), closes [#14](#14)
* test: Add unit tests ([75b30e0](75b30e0))
* test(commands): Test console descriptions (#16) ([e7ae82e](e7ae82e)), closes [#16](#16)
* test(coverage): Collect coverage from server tests ([ac7c963](ac7c963))
* test(server): Add reload command test ([ed85161](ed85161))
* chore(dev-branch): Alias develop branch with 0.4.x-dev ([df97b0b](df97b0b))
* chore(di): Make commands lazy-loaded ([ba947d8](ba947d8))
* chore(lock): Update composer.lock ([78b0aea](78b0aea))
* chore(server-tests): Fix script for use without code-coverage ([8a5580b](8a5580b))
* ci(travis): Bump test swoole version to 4.2.2 ([b0985ae](b0985ae))
* ci(travis): Test latest swoole ext version ([e99daba](e99daba))
* ci(travis): Validate composer files ([7dc4a5e](7dc4a5e))
* fix(command): Graceful shutdown ([7e6c9a4](7e6c9a4))
* feat(hmr): Implement HMR with Inotify ([97e88bb](97e88bb))

### BREAKING CHANGE

* - Server\HttpServerFactory should not be instantiated anymore, due to
removed hard coupling with ConfiguratorInterface, and `make()` method
becomig static. Now use directly: `HttpServerFactory::make()`
- Configuring server (using object implementing ConfiguratorInterface)
now happens in execute method of AbstractServerStartCommand
- Server\Configurator\ChainConfigurator now accepts
ConfiguratorInterface variadic starting from second argument and
implements IteratorAggregate retruning its configurators to ease DI usage (see
src/Bridge/Symfony/Bundle/Resources/commands.yaml)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant