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

MODULES-2210 Add TOS Parameter #282

Merged
merged 1 commit into from Jul 22, 2015
Merged

MODULES-2210 Add TOS Parameter #282

merged 1 commit into from Jul 22, 2015

Conversation

petems
Copy link

@petems petems commented Jul 13, 2015

Manycast Options
     tos [ceiling ceiling | cohort { 0 | 1 } | floor floor | minclock minclock
         | minsane minsane]
         This command affects the clock selection and clustering algo-
         rithms.  It can be used to select the quality and quantity of
         peers used to synchronize the system clock and is most useful in
         manycast mode.  The variables operate as follows:

$tos_minsane = 1
$tos_floor = 1
$tos_ceiling = 15
$tos_cohort = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the parameters other than $tos be exposed in class ntp?

Copy link
Author

Choose a reason for hiding this comment

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

Good point, adding those in now! 👍

@underscorgan
Copy link
Contributor

@petems so, one question above, but otherwise this needs documentation updates for the new parameter. Thanks!

@petems
Copy link
Author

petems commented Jul 14, 2015

@mhaskel Done! 👍

####`tos_cohort`

Specifies the cohort tos option. Valid options: '0' or '1'. Default value: varies by operating system

Copy link
Contributor

Choose a reason for hiding this comment

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

For all of these Default value: varies by operating system, it looks like params actually has a consistent default for all oses, so that should probably be listed instead.

Copy link
Author

Choose a reason for hiding this comment

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

Good point, will fix 👍

@underscorgan
Copy link
Contributor

Hey @petems ,

Looks like tests are failing now, and new tests should be added to cover the additional params I got you to add :)

Otherwise, just the doc updates, and if you could squash this down a little bit that would be awesome! Thanks!

@tphoney
Copy link
Contributor

tphoney commented Jul 21, 2015

Hi @petems , this change looks good, but would clutter the git history in its current state. Please squash them together and re-push here.

@petems
Copy link
Author

petems commented Jul 22, 2015

@tphoney Done! 👍

tphoney added a commit that referenced this pull request Jul 22, 2015
@tphoney tphoney merged commit 5317d44 into puppetlabs:master Jul 22, 2015
@tphoney
Copy link
Contributor

tphoney commented Jul 22, 2015

Great work @petems

@petems petems deleted the MODULES-2210-add_tos branch July 23, 2015 14:19
@petems
Copy link
Author

petems commented Jul 23, 2015

👍

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

4 participants