misc fixes #12
misc fixes #12
Conversation
The current default smbmetrics port number is 9922. Reflect this in top-level README file. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
internal/metrics/versions.go
Outdated
| } | ||
| return vers, nil | ||
| return vers, status |
There was a problem hiding this comment.
why not use a deferstatement for the return at the top of the function?
There was a problem hiding this comment.
Hmm, it looks to me like it would make an unnecessary complication to the code, and I don't think it is common by Go developers to use it for such cases.
There was a problem hiding this comment.
I am unclear on how defer would help here. I do find status a bit odd. Maybe consider a pattern like:
var err, innerErr error
vers.foo, innerErr = doThing()
err = errors.Join(err, innerErr)
vers.bar, innerErr = doAnotherThing()
err = errors.Join(err, innerErr)
/* ... and so on ... */
return vers, errI think that would simplify the function, but I won't demand it.
There was a problem hiding this comment.
Well, errors.Join was added in Go 1.20, and I don't know what the lowest version of Go the project is trying to support, but I think it's worth consideration still.
There was a problem hiding this comment.
Agreed. errors.Join would make the code cleaner. Will fix.
Do not bail-out in case of failure to resolve any of the sub-component versions but rather defer this failure to final status code. Required in order to avoid partial versions-view as part of exported status metric. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
Minor fixes as part of end-to-end testing with Samba, Ceph, cephadm and Promtheus.