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

winsvc: require serverengine #3866

Merged
merged 1 commit into from
Aug 22, 2022
Merged

winsvc: require serverengine #3866

merged 1 commit into from
Aug 22, 2022

Conversation

ashie
Copy link
Member

@ashie ashie commented Aug 22, 2022

Which issue(s) this PR fixes:
Follow up for #3849

What this PR does / why we need it:
In #3849, getting ruby's path was replaced with
ServerEngine.ruby_bin_path but require 'serverengine' was missing.
As a result it fails to launch fluentd service.

Docs Changes:
None

Release Note:
None (Same with #3849)

In #3849, getting ruby's path was replaced with
`ServerEngine.ruby_bin_path` but `require 'serverengine'` was missing.
As a result it fails to launch fluentd service.

Signed-off-by: Takuro Ashie <[email protected]>
@ashie ashie requested a review from cosmo0920 August 22, 2022 02:18
Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Thanks for the quick fix! 👍

@cosmo0920
Copy link
Contributor

BTW, can we add register/unregister e2e test on CI task? If not possible for now, testing manually is also OK for us.

@ashie
Copy link
Member Author

ashie commented Aug 22, 2022

I think at least service_main_start should be tested.
I'm considering how to test it, I'll post another PR if I done it.

@ashie ashie merged commit a2807c6 into master Aug 22, 2022
@ashie ashie deleted the follow-up-3849 branch August 22, 2022 04:36
@fujimotos fujimotos mentioned this pull request Aug 22, 2022
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