Parse /etc/os-release in diagnostics #2944
Conversation
vedxyz
left a comment
There was a problem hiding this comment.
I've intended to make a PR for the parent issue several times but still haven't gotten around to it. I would've liked to say I can help with the stuff I've left reviews for here, but not trusting myself on that currently.
a85c362 to
083ba5e
Compare
|
I sadly had to butcher the history (a lot of overlapping changes), but thanks for the review @vedxyz |
vedxyz
left a comment
There was a problem hiding this comment.
LGTM. Just two small style suggestions.
I'm still of the same opinion as my earlier comment regarding dictionary vs class, but no biggie. @X9VoiD said he might have another look anyway so he might go for it if he sees fit.
Co-authored-by: Ved <dev@vedat.xyz>
There is nothing particularly interesting in the OperatingSystem struct at the moment, so I see no need to complicate the class with a serialization function. |
|
Pushed commits to this PR. It looks like this now on Linux: "OperatingSystem": {
"Name": "Fedora Linux",
"Version": "38 (Workstation Edition)",
"Attributes": {
"ID": "fedora",
"VERSION_ID": "38",
"VERSION_CODENAME": "",
"PRETTY_NAME": "Fedora Linux 38 (Workstation Edition)",
"SUPPORT_END": "2024-05-14",
"VARIANT": "Workstation Edition",
"VARIANT_ID": "workstation",
"KERNEL_VERSION": "6.4.12-200.fc38.x86_64"
}
},and this on Windows: "OperatingSystem": {
"Name": "Win32NT",
"Version": "10.0.22621.0",
"Attributes": {
"ServicePack": ""
}
}, |
Closes #2744
Supersedes #2819
Pre-PR diagnostics on Linux:
See #2944 (comment) for updated changes
Post-PR diagnostics on Arch Linux removedAllos-releasepairs are added, except for a (small) key blacklist (currentlyLOGOandANSI_COLOR) as well as any key containingURL.On non-Linux there is a very small (likely wanted) side-effect of having the Platform field be correctly stringified - previously it would post e.g."Platform": 4but now it posts"Platform": "Unix"Code or naming is probably messy, reviews are very welcome in those areas.