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 ability for service to report the system managing the service #147

Merged
merged 4 commits into from Sep 10, 2018

Conversation

Projects
None yet
3 participants
@SteelPhase
Contributor

SteelPhase commented Sep 10, 2018

This will only matter in instances where you may not be using service.New, or service.ChosenSystem. This allows the service itself to report the system it was created with.

@coveralls

This comment has been minimized.

coveralls commented Sep 10, 2018

Coverage Status

Coverage decreased (-0.01%) to 9.442% when pulling c436c0b on SteelPhase:system-name into 8f267d8 on kardianos:master.

@kardianos

This comment has been minimized.

Owner

kardianos commented Sep 10, 2018

What's the difference between service.Platform() and service.Service.SystemName() ? Is there a reason to not have this as a package global function? I guess I'm not sure how it could be different from one service to another service (on a single run).

@SteelPhase

This comment has been minimized.

Contributor

SteelPhase commented Sep 10, 2018

I ran into a case today where I was passing service.Service around and needed to know what Platform that service was. I went with this approach because it seemed pretty simple. Since my use case can manage multiple different services at once, I have situations where a service could be running on SystemV or Upstart on the same box.

service.go Outdated
@@ -347,6 +347,9 @@ type Service interface {
// otherwise the name.
String() string
// SystemName displays the name of the system that will be used to manage the service.

This comment has been minimized.

@kardianos

kardianos Sep 10, 2018

Owner

// SystemName displays the name of the system that manages the service.
// In most cases this will be the same as Platform().

LGTM with these changes.

This comment has been minimized.

@SteelPhase

SteelPhase Sep 10, 2018

Contributor

Should I switch SystemName to Platform to keep it inline with the rest of the package?

This comment has been minimized.

@kardianos

@kardianos kardianos merged commit b1866cf into kardianos:master Sep 10, 2018

1 check failed

coverage/coveralls Coverage decreased (-0.01%) to 9.442%
Details

@SteelPhase SteelPhase deleted the SteelPhase:system-name branch Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment