Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Oct 24, 2017

I copied over ConsoleRunHost from topshelf and modified it to work with
DotNetCore.WindowsService library. The console service host blocks until
the user hits the CancelKey(Ctrl+Break or Ctrl+C). The ExampleService.cs
is updated to reflect the library update. The update allows for
debugging of services which are push based rather than pull based.

I copied over ConsoleRunHost from topshelf and modified it to work with
DotNetCore.WindowsService library. The console service host blocks until
the user hits the CancelKey(Ctrl+Break or Ctrl+C). The ExampleService.cs
is updated to reflect the library update. The update allows for
debugging of services which are push based rather than pull based.
@japerr
Copy link
Contributor

japerr commented Oct 25, 2017

This is my PR, I combined accounts ghost == japerr

namespace PeterKottas.DotNetCore.WindowsService.Example
{
public class ExampleService : IMicroService
public class ExampleService : IMicroService
Copy link
Owner

Choose a reason for hiding this comment

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

It seems your settings for tabs are different to what the rest of the code bases uses, would you pls be able to reformat this to match existing formating?

Copy link
Contributor

@japerr japerr Oct 26, 2017

Choose a reason for hiding this comment

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

To help prevent this in the future, I added a solution level .editorconfig with formatting. EditorConfig standards are baked into VS2017. For *.cs files the indent style is set to space and the indent size is 4.

public class ExampleService : IMicroService
{
private IMicroServiceController controller;
private IMicroServiceController _controller;
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 pls rename this back? I know it's common naming convention but it's not really used anywhere else so it would be breaking what we have there atm.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem

serviceHost.Run();
break;
case ActionEnum.RunInteractive:
Start(config);
Copy link
Owner

Choose a reason for hiding this comment

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

Why do we want this gone? Just checking if it won't break some of functionality people are already using.

Copy link
Contributor

Choose a reason for hiding this comment

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

That line is replaced by lines 141 - 158 in ServiceRunner.cs. I moved the actual execute into ServiceRunner.Run so the new ConsoleServiceHost result is returned as the result of ServiceRunner.Run

@PeterKottas
Copy link
Owner

Thanks a lot for this mate! Definitely looks good, there's just a few small things I noticed and left comments so that you could take a look at it. Cheers ;)

@japerr
Copy link
Contributor

japerr commented Oct 26, 2017

Updated PR reflect code review changes

@japerr
Copy link
Contributor

japerr commented Nov 1, 2017

This has been updated to reflect the code review changes.

@PeterKottas
Copy link
Owner

Hey mate, sorry been busy with some stuff. Going to merge and push that out now. Thanks again!

@PeterKottas PeterKottas merged commit a85dd47 into PeterKottas:master Nov 1, 2017
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