-
-
Notifications
You must be signed in to change notification settings - Fork 862
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
Suppress MacSysctl Warning Logs #2212
Conversation
|
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.
Code looks great with a few document requests. Can you also add a change log entry?
// 64-bit flag | ||
private static final int P_LP64 = 0x4; | ||
/* | ||
* macOS States: | ||
* macOS States: 7 |
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.
What's this 7 for?
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 think by mistake it gets typed.
# When operating without elevated permissions, this results in frequent error | ||
# messages for failures to read the process environment files. Set this to true | ||
# to receive these warnings. | ||
oshi.os.linux.procfs.logwarning=false | ||
|
||
#Macos gives log warning if SYSCTL calls fails | ||
#In order to avoid warning make this property true | ||
|
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.
Since the reasoning for this is the same as Linux (no elevated permissions) can we merge the two paragraphs above? For example (feel free to use different wording) insert a comment right below line 119, # On macOS, process environment is read via sysctl
. And then you can put the two settings next to each other.
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.
Ya thanks for the suggesstion.
Thanks for your contribution! Fantastic job. I'm off for the night but other than the minor comment changes should be good to merge tomorrow. |
Yes |
Resolves #2206 |
* Added oshi.os.mac.sysctl.logwarning property * Superess the sysctl log warning * Added code to read oshi.os.mac.sysctl.logwarning * Minor Comment Changes * Added #2212 to changelog * Added #2212 to changelog * Added #2212 to changelog * Removed 7 * Spotless * Change log tweaks Co-authored-by: Daniel Widdis <widdis@gmail.com>
No description provided.