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

fix leap_status value in chrony input plugin #1983

Merged
merged 1 commit into from
Nov 3, 2016

Conversation

m4ce
Copy link
Contributor

@m4ce m4ce commented Nov 2, 2016

see #1982

@sparrc
Copy link
Contributor

sparrc commented Nov 2, 2016

please fix the unit tests

@m4ce
Copy link
Contributor Author

m4ce commented Nov 2, 2016

@sparrc, I must be missing something here but I don't see how my change would affect the output expected by the unit test? I see leap_status is expected to be set to "normal" and that's what it is set to. If you could point me out to the reason, I will be happy to fix it.

@sparrc
Copy link
Contributor

sparrc commented Nov 2, 2016

@m4ce are you able to run the unit tests locally? your changes appear to be changing some exissting metrics, I don't have time to dig too deeply but I think if you can run the unit tests you can figure out what's changed

@m4ce
Copy link
Contributor Author

m4ce commented Nov 2, 2016

@sparrc, no worries, I had a better look and the problem is the reference_id field. I've now commited a fix for it, let's wait for the unit test to finish.

@m4ce m4ce force-pushed the issue-1982 branch 2 times, most recently from 18612b0 to ee2562a Compare November 2, 2016 16:45
@sparrc
Copy link
Contributor

sparrc commented Nov 2, 2016

@m4ce can you add a unit test to verify that Leap Status: Not synchronised is getting represented properly now?

@sparrc sparrc added this to the 1.2.0 milestone Nov 2, 2016
@m4ce
Copy link
Contributor Author

m4ce commented Nov 2, 2016

@sparrc i've done that, i hope it looks good now.

@sparrc sparrc merged commit e43cfc2 into influxdata:master Nov 3, 2016
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