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

User config #10

Merged
merged 2 commits into from
Dec 2, 2016
Merged

Conversation

lightonflux
Copy link
Contributor

Fix #9

@lightonflux lightonflux mentioned this pull request Nov 10, 2016
4 tasks
@VojtechMyslivec
Copy link
Owner

Thanks for PR. I generally like that idea and will take a look at it soon.

@lightonflux
Copy link
Contributor Author

Did you have time to look at it. Do you want me to redo it for the zimbra-v8.7_dev branch?

@VojtechMyslivec VojtechMyslivec changed the base branch from master to dev December 2, 2016 12:25
@@ -0,0 +1,2 @@
*~
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer this in your personal gitignore, but alright, why not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It excludes temporary saves from some text editors. Never heard of someone having a problem with this rule in production. Never saw filename~ used for anything else than temporary files.

Copy link
Owner

Choose a reason for hiding this comment

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

If your editor does it, it should be in your personal gitignore. Otherwise you can have it in all of your repositories. I have configured my vim to save it to ~/.vim/backup to not spam the file system with bogus files.

But whatever, I merged it.

@VojtechMyslivec VojtechMyslivec merged commit 3af64bc into VojtechMyslivec:dev Dec 2, 2016
@VojtechMyslivec
Copy link
Owner

I merge the changes to dev branch (and amend it in ea2e0e3).

Thanks for PR. Please, review my merge changes - they are mentioned in the commit message.

@lightonflux
Copy link
Contributor Author

I would like to integrate the domains and email into the config so that you don't need to change cron or your systemd timer if you change these options.

E.g.

 domains="mail.example.com www.mail.example.com mx.example.com"
 email="admin@example.com"

I am not sure how to integrate that into the script. I don't see how you handle $2 etc.

@VojtechMyslivec
Copy link
Owner

You're right. I had some ideas but it doesn't make sense anymore.

It should be in config file and there are no needs for script parameters anymore.

@VojtechMyslivec
Copy link
Owner

Hi @lightonflux I moved the email and common name to the config file in 8c98b4e

It is in parameter branch at the moment. I will merge it soon into dev.

@lightonflux lightonflux deleted the user-config branch December 10, 2016 20:33
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.

None yet

2 participants