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

Add Unit socket #27

Merged
merged 1 commit into from
Apr 25, 2019
Merged

Add Unit socket #27

merged 1 commit into from
Apr 25, 2019

Conversation

peter279k
Copy link
Contributor

Changed log

Copy link
Owner

@icanhazstring icanhazstring left a comment

Choose a reason for hiding this comment

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

$commandDispatcher = $this->createCommandDispatcherStub();
$commandDispatcher->dispatch(Argument::cetera())->willReturn($command);

$timer = new Socket('AwesomeService', $commandDispatcher->reveal());
Copy link
Owner

Choose a reason for hiding this comment

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

Please rename as $socket

$commandDispatcher = $this->createCommandDispatcherStub();
$commandDispatcher->dispatch(Argument::cetera())->willThrow(CommandFailedException::class);

$timer = new Socket('AwesomeSocket', $commandDispatcher->reveal());
Copy link
Owner

Choose a reason for hiding this comment

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

Rename as $socket as well

namespace SystemCtl\Unit;

/**
* Class Timer
Copy link
Owner

Choose a reason for hiding this comment

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

Class Socket please

@peter279k
Copy link
Contributor Author

@icanhazstring, thank you for your request changes.

And I also appreciate your review.

I've accepted request changes that you mention in the latest commit.

Please check that. If having any request changes, feel free to write related comments on that line!

Thanks :-).

Copy link
Owner

@icanhazstring icanhazstring left a comment

Choose a reason for hiding this comment

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

Just a small typo and string value change.
Thanks for the adjustments!

$commandDispatcher = $this->createCommandDispatcherStub();
$commandDispatcher->dispatch(Argument::cetera())->willReturn($command);

$socket = new Socket('AwesomeService', $commandDispatcher->reveal());
Copy link
Owner

Choose a reason for hiding this comment

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

Could you name it AwesomeSocket please?

/**
* @test
*/
public function itShouldThrowAnExeceptionIfNotSocketCouldBeFound()
Copy link
Owner

Choose a reason for hiding this comment

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

Just a minor typo here
...ExceptionIfNoSocketCouldBeFound

@peter279k
Copy link
Contributor Author

@icanhazstring, thank you for your comment about request changes.

Now I've already accepted latest request changes and push the latest commit.

Thanks.

@icanhazstring
Copy link
Owner

Hi @peter279k thanks for the changes. Merged! :)

@icanhazstring icanhazstring merged commit 4f2299c into icanhazstring:master Apr 25, 2019
@peter279k peter279k deleted the issue_#6 branch April 29, 2020 17:18
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

2 participants