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

Adapt to use new service widget #49

Merged
merged 6 commits into from
Aug 9, 2018

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Aug 8, 2018

@coveralls
Copy link

coveralls commented Aug 8, 2018

Coverage Status

Coverage increased (+0.8%) to 3.596% when pulling b035f54 on joseivanlopez:adapt_service_widget into c7e321c on yast:master.

Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

Please, update the yast2 dependency.

# for backward compatibility, see {#write_daemon}. When the service
# is configured by using the UI, it directly saves the service, see
# {Yast2::SystemService#save}.
def save_status
Copy link
Member

@dgdavid dgdavid Aug 8, 2018

Choose a reason for hiding this comment

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

In the http-sever I used save_service for an identical method. What do you about use the same naming, when possible, for the "service widget adaptation"?

I don't mind to change save_service to save_status if you think it is more accurate :)

Copy link
Contributor Author

@joseivanlopez joseivanlopez Aug 9, 2018

Choose a reason for hiding this comment

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

Well, in this case I named it #save_status because it saves the service status in different ways depending on the mode. It does not always "save the service", understanding "service" is our SystemService.

@joseivanlopez joseivanlopez merged commit 5cdd602 into yast:master Aug 9, 2018
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.

4 participants