Skip to content

Adding ppolicy #5

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

Merged
merged 1 commit into from
Feb 16, 2016
Merged

Adding ppolicy #5

merged 1 commit into from
Feb 16, 2016

Conversation

jsdevel
Copy link
Contributor

@jsdevel jsdevel commented Dec 11, 2015

No description provided.

@jsdevel
Copy link
Contributor Author

jsdevel commented Feb 7, 2016

@dinkel thoughts on this PR?

@dinkel
Copy link
Owner

dinkel commented Feb 7, 2016

Hi @jsdevel,

Oops, somehow I missed this PR, sorry. I will have a look at it in the next few days, but already thanks a lot for your contribution.

@jsdevel
Copy link
Contributor Author

jsdevel commented Feb 8, 2016

No worries! Curious to know your thoughts on the addition. I've been using ppolicy successfully with this fork. I'd like to see it get merged though so I can unpublish my version!

@dinkel dinkel merged commit 88614bd into dinkel:master Feb 16, 2016
@jsdevel jsdevel deleted the adding-ppolicy branch February 16, 2016 21:54
@jsdevel
Copy link
Contributor Author

jsdevel commented Feb 16, 2016

Thanks @dinkel !

@dinkel
Copy link
Owner

dinkel commented Feb 16, 2016

Hi Joseph,

After reading into ppolicy and finally finding time I did merge your pull request. Thanks a lot for that!

I made some minor changes in the entrypoint.sh to keep a consistent coding style (although I do like your pipes in the sed arguments ;-)).

In the documentation in README.md I liked the bulletpoint style for the environment variable definitions, but I prefer "======" and "-----" underlining of headings. Did you have a special reason to change this? While learning about ppolicy I did move and rewrite parts of the ppolicy configuration instructions to hopefully be more clear.

I would appreciate if you had a look at the merged version and tell me if this fits your use.

Anyways thanks again for the contribution.

@jsdevel
Copy link
Contributor Author

jsdevel commented Feb 16, 2016

Your changes look fine.

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