-
Notifications
You must be signed in to change notification settings - Fork 51
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
Cleanup errors caught by megacheck #85
Conversation
Signed-Off-By: Joe Handzik <joseph.t.handzik@hpe.com>
sources/procfs.go
Outdated
for _, item := range jobList { | ||
metricList = append(metricList, item) | ||
} | ||
metricList = append(metricList, jobList...) |
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.
Do we want (need?) to check if jobList
is not nil
before this line? Say there aren't any jobs in the job_stats
file (happens whenever no jobs have been running for a period of time), the for loop on line 611 would have zero iterations and jobList
would never be initialized. I'm not sure how the append(...)
command handles nil
elements. If it handles them gracefully, no issue, otherwise we might want to add a check beforehand.
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.
Looks good outside of my comments about the append
functions. Once we get those figured out I can merge!
for _, item := range statsList { | ||
metricList = append(metricList, item) | ||
} | ||
metricList = append(metricList, statsList...) |
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.
See my other comment - we do the opposite here. Might be good to stay consistent.
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.
Sorry for the double review, the second comment wasn't added to the first one for some reason.
103d19b
to
506fd57
Compare
@roclark PR, now with 100% more consistency. |
Signed-Off-By: Joe Handzik joseph.t.handzik@hpe.com