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.

Sign up for GitHub

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

Jump to bottom

Prevent creation of pid file directory #531

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

Prevent creation of pid file directory #531

wants to merge 1 commit into from

Conversation

TuningYourCode
Copy link
Contributor

Pull Request (PR) description

This PR prevents the creation of the hardcoded pid file directory if it is not used.

This Pull Request (PR) fixes the following issues

Fixes #501

@bastelfreak bastelfreak added bug Something isn't working needs-feedback Further information is requested labels Aug 18, 2019
@TuningYourCode TuningYourCode changed the title Prevent creation of pid file directory if not used Prevent creation of pid file directory Aug 29, 2019
@TuningYourCode
Copy link
Contributor Author

@bastelfreak any feedback on this PR or should we just close and leave the code as it is?

ghoneycutt
ghoneycutt reviewed Nov 4, 2019
View reviewed changes
manifests/fpm/config.pp Outdated Show resolved Hide resolved
@vox-pupuli-tasks
Copy link

Dear @TuningYourCode, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@TuningYourCode
Copy link
Contributor Author

Hi @bastelfreak & @ghoneycutt,

how should we proceed with this PR? If there are any todos please tell me :)
Feel free to close this PR if you do not like to merge it.

@ghoneycutt
Copy link
Member

Please change to only remove the pid file directory and I'm +1 for merging.

@TuningYourCode
Copy link
Contributor Author

Restored the logic to manage the log directory :)

@bastelfreak
Copy link
Member

@TuningYourCode there are merge conflicts and they must be resolved before we can review and merge this.

@vox-pupuli-tasks
Copy link

Dear @TuningYourCode, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@TuningYourCode
Copy link
Contributor Author

Not sure why these tests fail, they do not check for presents of the removed definition.

@vox-pupuli-tasks
Copy link

Dear @TuningYourCode, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

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

@bastelfreak bastelfreak bastelfreak left review comments

@ghoneycutt ghoneycutt ghoneycutt left review comments

Assignees
No one assigned
Labels
bug Something isn't working merge-conflicts needs-feedback Further information is requested needs-tests tests-fail
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

undesired service restart due to missing /var/run/php-fpm dir
3 participants
@TuningYourCode @ghoneycutt @bastelfreak