-
Notifications
You must be signed in to change notification settings - Fork 4
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
Perfdata errors #116
Perfdata errors #116
Conversation
This patch changes the function signature of Performance data formatting functions to return errors. This is a preparation step to allow the rejection of certain values, which are valid values for that data type, but not valid as measurements
closes #115 |
Nice. I'll have a look at it later today |
perfdata/type.go
Outdated
// Returns an eror in some known cases where the value of a data type does not | ||
// represent a valid measurement, see the explanation for "formatNumeric" for | ||
// perfdata values. | ||
func (p Perfdata) String() (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that this is no longer implements the Stringer interface. https://pkg.go.dev/fmt#Stringer
I rather have the formatNumeric handle that and the Stringer interface be implemented, since it is more expectable when having a String() method on a struct.
Not sure how else we could or should communicate that NaN/Inf are not printed. Maybe just docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point. I think I have an idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I restored the behaviour of String
by using the new functionality and just ignoring errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coolio. I'll have a look later.
This commit restores the previous signature of String() for perfdata by moving the functionality in a new method (which is also exported). This new method validates the data and throws errors, the String() method catches them and just returns the empty string then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be merged.
We can still have a discussion on if and how to allow strings a values later
No description provided.