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

Enable/Disable services #17

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Enable/Disable services #17

wants to merge 2 commits into from

Conversation

passie
Copy link

@passie passie commented Mar 6, 2020

I needed a way to disable servers which I didn't need.
The following three services are able to be turned off.

  • dovecot
  • exim
  • named

@ju5t
Copy link
Contributor

ju5t commented Mar 8, 2020

Thanks for the PR.

At the moment, the PR doesn't follow our current design pattern. All of our variables are defined at the top level. And although they could arguable be put elsewhere, I believe it's better to hold onto one standard, albeit a little outdated, then to put them all over the place.

Wouldn't it be better to manage the service itself then to not manage it entirely? For example, you could ensure the service is not running if that is what you prefer. However, if that is the case, this isn't 100% complete yet as you need to change /usr/local/directadmin/data/admin/services.status too. Without it, the service check will run and turns it back on again.

We're also missing some tests and documentation. We require both to merge this PR.

@ju5t ju5t changed the base branch from master to develop March 8, 2020 13:34
@passie
Copy link
Author

passie commented Mar 14, 2020

Thanks for your feedback.

I am currently modifying my code, which will hopefully cover your comments. I will update the code first and if you like it I will adjust the rspec and documentation once you are satisfied with the code.

Hopefully your okay with this?

@ju5t
Copy link
Contributor

ju5t commented Mar 16, 2020

Sounds good!

@mkjmdski
Copy link

mkjmdski commented Apr 4, 2020

What is the status of this PR? It looks very usefull though!

@passie
Copy link
Author

passie commented Apr 19, 2020

@mkjmdski Sorry for the late answer, although I am still willing to make the changes to the code so that it meets the standards required by this module, Corona has delayed some things.
therefore I'm not sure when the code will be finished.

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.

3 participants