search

geerlingguy / ansible-role-ntp

Ansible Role - NTP
https://galaxy.ansible.com/geerlingguy/ntp/
MIT License
319 stars 243 forks source link

unnecessary variable definiton for redhat #80

Closed vmpr closed 3 years ago

vmpr commented 4 years ago

Hi Jeff, thanks for your hard work and maintaining so many ansible roles!

I found something in this role, one variable ( ntp_driftfile ) is not necessary in RedHat.yml:

cat roles/galaxy/geerlingguy.ntp/vars/RedHat.yml
---
__ntp_daemon: chronyd
ntp_tzdata_package: tzdata
__ntp_package: chrony
__ntp_config_file: /etc/chrony.conf
ntp_driftfile: /var/lib/ntp/drift
ntp_cron_daemon: crond

For Redhat 7+ is only chrony relevant and in the chrony template file there is no variable "ntp_driftfile" :

cat roles/galaxy/geerlingguy.ntp/templates/chrony.conf.j2
...
# Record the rate at which the system clock gains/losses time.
driftfile /var/lib/chrony/drift
...

it would also has the wrong value :)

I am happy to do a PR if we are on the same page? thanks!

stale[bot] commented 4 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

vmpr commented 4 years ago

I would like to fix that issue for everyone, will you merge a PR? I know everyone is busy especially with all your youtube recording, but three issues I've filed just got staled - would be nice to fix them!

stale[bot] commented 4 years ago

This issue is no longer marked for closure.

stale[bot] commented 3 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

vmpr commented 3 years ago

I would like to fix that issue for everyone, will you merge a PR? I know everyone is busy especially with all your youtube recording, but three issues I've filed just got staled - would be nice to fix them!

stale[bot] commented 3 years ago

This issue is no longer marked for closure.

stale[bot] commented 3 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

vmpr commented 3 years ago

again, happy to fix that if u find time to answer @geerlingguy

stale[bot] commented 3 years ago

This issue is no longer marked for closure.

geerlingguy commented 3 years ago

If you would like to file a PR that works on RHEL and doesn't break existing installs on other platforms I'd be willing to take a look. Sorry for the delays :)

stale[bot] commented 3 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

stale[bot] commented 3 years ago

This issue has been closed due to inactivity. If you feel this is in error, please reopen the issue or file a new issue with the relevant details.