-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Include Linux DE in OS sysinfo string. #20917
Conversation
Squeezing everything into one string won't make evaluations easier. |
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.
LGTM, untested
if (!string.IsNullOrEmpty(sessionType)) | ||
sessionType = $" ({sessionType})"; | ||
|
||
string suffix; |
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.
string suffix; | |
var suffix = string.Empty; |
and then drop the else
case.
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 the current setup makes more sense. (Why assign values which will get overwritten anyway?)
Looks fine from my end. |
This PR adds the current desktop environment alongside the OS and display server on Linux. This can inform discussions about whether / how much effort to put into supporting a given DE is worthwhile.
From the Arch wiki: