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

README: Add explanation of settings.ntp.time-servers #550

Merged
merged 1 commit into from
Nov 21, 2019

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Nov 21, 2019

Testing done:

Rendered version looks OK.

@iliana
Copy link
Contributor

iliana commented Nov 21, 2019

"URIs" seems weird to me because I expect only domain names to work for chrony.

@tjkirch
Copy link
Contributor Author

tjkirch commented Nov 21, 2019

"URIs" seems weird to me because I expect only domain names to work for chrony.

Or IPs.

I suppose this is following the implementation in #549, which improved the accuracy of the NTP server list from "any single-line string" to a URI, and so this documents it. I assume you'd prefer a modeled type that only allows an IP or hostname, with or without an "ntp://" scheme perhaps?

@iliana
Copy link
Contributor

iliana commented Nov 21, 2019

I don't really care about how it's modeled/validated (and "URI-like" seems totally fine to me). I just think the docs should say what most folks are going to expect an NTP configuration to want (domains and/or IPs).

@tjkirch
Copy link
Contributor Author

tjkirch commented Nov 21, 2019

I just think the docs should say what most folks are going to expect an NTP configuration to want

Perhaps just "A list of NTP servers" then?

@iliana
Copy link
Contributor

iliana commented Nov 21, 2019

Sounds good to me 👍

@tjkirch
Copy link
Contributor Author

tjkirch commented Nov 21, 2019

This push simplifies the description to "A list of NTP servers ..."

@tjkirch tjkirch merged commit 3324edf into develop Nov 21, 2019
@tjkirch tjkirch deleted the doc-time-servers branch November 21, 2019 19:31
@iliana iliana added this to the v0.2.0 milestone Nov 21, 2019
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