Skip to content
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.

Sign up for GitHub

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

Jump to bottom

OSDescription.Linux: return a user-friendly name based on /etc/os-release. #83976

Merged
merged 6 commits into from
Apr 18, 2023
Merged

OSDescription.Linux: return a user-friendly name based on /etc/os-release. #83976

merged 6 commits into from
Apr 18, 2023

Conversation

tmds
Copy link
Member

@tmds tmds commented Mar 27, 2023 •
edited
Loading

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 27, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Mar 27, 2023
@build-analysis build-analysis bot mentioned this pull request Mar 27, 2023
Closed
@MichalPetryka
Copy link
Contributor

Should a fallback based on lsb_release be added here?

@tmds
Copy link
Member Author

tmds commented Mar 27, 2023

Should a fallback based on lsb_release be added here?

It can be added in a follow-up PR.
Do you have a specific distro in mind?

@MichalPetryka
Copy link
Contributor

Do you have a specific distro in mind?

I was thinking about Alpine here.

@tmds
Copy link
Member Author

tmds commented Mar 27, 2023

I was thinking about Alpine here.

Though it doesn't use systemd, Alpine does include an /etc/os-release file.

@am11
Copy link
Member

am11 commented Mar 28, 2023

Fixes #37923.

I don't think this is addressing #37923.

@tmds
Copy link
Member Author

tmds commented Mar 28, 2023

I don't think this is addressing #37923.

ok, I've removed it.

@teo-tsirpanis teo-tsirpanis added area-System.Runtime.InteropServices and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 28, 2023
@ghost
Copy link

ghost commented Mar 28, 2023

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #83287.

@am11 @tannergooding @danmoseley @stephentoub ptal.

cc @richlander

Author: tmds
Assignees: -
Labels:

area-System.Runtime.InteropServices, community-contribution
Milestone: -

@build-analysis build-analysis bot mentioned this pull request Mar 28, 2023
Closed
@richlander
Copy link
Member

Thanks for taking this on.

@tmds tmds closed this Apr 13, 2023
@tmds tmds reopened this Apr 13, 2023
@tmds
Copy link
Member Author

tmds commented Apr 13, 2023

CI looks good for this. This is up for review.

tmds and others added 2 commits April 13, 2023 16:11
@tmds
Copy link
Member Author

tmds commented Apr 18, 2023

Is this good to merge?

stephentoub
stephentoub reviewed Apr 18, 2023
View reviewed changes
src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs
}
}

return null;
Copy link
Member

@stephentoub stephentoub Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd find this easier to read if this were moved up to above the static local function.

stephentoub
stephentoub approved these changes Apr 18, 2023
View reviewed changes
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@stephentoub stephentoub merged commit 9672d82 into dotnet:main Apr 18, 2023
@tmds tmds mentioned this pull request Apr 19, 2023
Merged
@ghost ghost locked as resolved and limited conversation to collaborators May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@omajid omajid omajid left review comments

@danmoseley danmoseley danmoseley left review comments

@teo-tsirpanis teo-tsirpanis teo-tsirpanis left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees
No one assigned
Labels
area-System.Runtime.InteropServices community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RuntimeInformation.OSDescription is really KernelDescription
8 participants
@tmds @MichalPetryka @am11 @richlander @omajid @stephentoub @danmoseley @teo-tsirpanis