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

call service.Context() after service.Init() on windows #29

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ploxiln
Copy link

@ploxiln ploxiln commented Aug 15, 2021

service.Context() is called after Init() (and after Start()) on unixes,
so some users did not expect or test with Context() called before Init()

this was noticed in nsqio/nsq#1359
and is still needed along with nsqio/nsq#1361

cc @mreiferson @judwhite

service.Context() is called after Init() (and after Start()) on unixes,
so some users did not expect or test with Context() called before Init()
@mreiferson
Copy link

Ahh right, thanks.

@ploxiln
Copy link
Author

ploxiln commented Aug 15, 2021

It would make sense to also move the call to service.Context() earlier in the unix variant, so it's also before Start() and after Init(), but I decided to leave this PR more minimal for now.

... and also that's theoretically a backwards-compatibility break for unix programs using go-svc that depended on Context() being called after Start() ... but it seems pretty unlikely that some project would use go-svc but not care that the windows case is broken ;)

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