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.

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

Update RIDs for uniformity, remove unsupported OS #8710

Merged
merged 7 commits into from
Feb 23, 2019
Merged

Update RIDs for uniformity, remove unsupported OS #8710

merged 7 commits into from
Feb 23, 2019

Conversation

ErikNoren
Copy link
Contributor

Minor changes to update the RIDs and add convenience comments for users to better understand that the list is not exhaustive and there are potentially more targets available. Updated formatting to keep consistency.

I didn't update the Android section to include a Portable entry since I'm not sure if android is portable with the .NET Core 2.0 minimum requirement. The macOS section is more cluttered with the reformatting so I'd be open to compressing it again but it looks odd having a Portable option with indented RID and the others being at the Portable level. I also liked including the OS names which people typically remember more than they do the numeric representation.

For macOS 10.10 Yosemite I don't see it listed as supported or unsupported but it was in the original document and exists in the runtime.json file so I did not remove it but I wonder if it should be removed.

Summary

Updated the Using RIDs section and those below.

  • Removed comment about omitting some architectures so the linux-arm target could be included (for Raspberry Pi reference below).
  • Added a comment below each OS section to repeat the note that the list is not complete and includes a link to the JSON file with all targets.
  • Added a note below Linux RIDs to mention Raspberry Pi specifically since some users do not understand the distinction between OS and architecture as evidenced by issues and questions I get when doing presentations on .NET Core.
  • Alpha-sorted the Linux distributions as the list has gotten long and looked as though it was intended to be alpha-sorted. This improves discoverability of available targets. The sorting is based on the name, not the RID.
  • Removed Ubuntu 17.10 as it's marked as no longer supported.
  • Reformatted the macOS section to mirror the other sections including the addition of a Portable section.
  • All Portable sections got a note explicitly that they are supported in .NET Core 2.0 and later.

Updated the RID list to provide a link to the larger json document in each section for those who jump around and may not read the paragraph leading into the section (this is backed by issues/comments from others who read this as an exhaustive list). Removed Ubuntu 17.10 as it's no longer supported. Reformatted the macOS section to be more consistent with the others by listing each OS version separate and included a "portable" target. Added a note to each "portable" entry to denote it's usable with .NET Core 2.0 or above.
@ErikNoren ErikNoren requested a review from mairaw as a code owner October 28, 2018 21:33
@dnfclas
Copy link

dnfclas commented Oct 28, 2018

CLA assistant check
All CLA requirements met.

@rpetrusha
Copy link
Contributor

//cc @terrajobst

@terrajobst
Copy link
Member

I think @richlander would be more qualified to review this.

@rpetrusha rpetrusha requested a review from richlander November 7, 2018 00:52
@jkotas jkotas mentioned this pull request Nov 13, 2018
docs/core/rid-catalog.md Outdated Show resolved Hide resolved
docs/core/rid-catalog.md Outdated Show resolved Hide resolved
docs/core/rid-catalog.md Outdated Show resolved Hide resolved
@richlander
Copy link
Member

@ErikNoren - @jkotas left some good feedback. Can share your plans to resolve that?

@ErikNoren
Copy link
Contributor Author

@richlander I saw the comments and can absolutely make the changes but they came just before I started international travel with severely limited access. I will review again when I return home the second week of December. I will check to see if anything is announced at Connect(); that would impact the changes at that time as well.

@ErikNoren
Copy link
Contributor Author

I've made the changes I understood and resolved the conversations on those (I hope that was right; I didn't find any documentation guidance on what to do with those) but left open 2 where I have a question or want review feedback. This is my first time doing a pull for such a large, active repository so I apologize for mistakes I might make.

docs/core/rid-catalog.md Outdated Show resolved Hide resolved
@ErikNoren
Copy link
Contributor Author

All review items from @jkotas have been incorporated and committed.

@ErikNoren
Copy link
Contributor Author

Is there anything else I need to do? This is my first contribution and I'm not sure if I'm all done and this is waiting for some final approval to pull or if I was supposed to do something else. @richlander

Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

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

Sorry we dropped the ball here @ErikNoren. I lost track of this PR during the holidays. Can you resolve the merge conflicts and also remove the metadata that adds my name here at the top? I'm assigned to do this at a global level.

@jkotas do we need anyone else to review this? Also, is the removal of rhel.7-x64 intentional?

@@ -1,6 +1,9 @@
---
title: .NET Core Runtime IDentifier (RID) catalog
description: Learn about the Runtime IDentifier (RID) and how RIDs are used in .NET Core.

Copy link
Contributor

Choose a reason for hiding this comment

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

remove these new values

@@ -1,6 +1,9 @@
---
title: .NET Core Runtime IDentifier (RID) catalog
description: Learn about the Runtime IDentifier (RID) and how RIDs are used in .NET Core.

author: mairaw
ms.author: mairaw
ms.date: 07/19/2018
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ms.date: 07/19/2018
ms.date: 02/22/2019

@jkotas
Copy link
Member

jkotas commented Feb 23, 2019

@jkotas do we need anyone else to review this?

@richlander may want to take a look.

Also, is the removal of rhel.7-x64 intentional?

Yes, we have kept only the common values that are likely to be used in practice:

  • rhel.7-x64 is equivalent to linux-x64. We recommend to use linux-x64 as much as possible, and not really use distro specific RIDs like rhel.7-x64.
  • rhel.6-x64 is not equivalent to linux-x64. It is why it was kept as common value.

Note that all the RIDs that have been removed from the list in this doc are still in the big .json file with the whole RID graph linked from the docs.

@mairaw
Copy link
Contributor

mairaw commented Feb 23, 2019

Cool. So I'll resolve the merge conflicts and update the metadata and merge this. Thanks everyone!

@mairaw mairaw merged commit 639902b into dotnet:master Feb 23, 2019
Blind-Striker added a commit to Blind-Striker/Cake.Electron.Net that referenced this pull request Sep 7, 2019
* New electronize add command support added
* Some rid removed and added based on new .NET Core RID catalog. See dotnet/docs#8710 and https://github.com/dotnet/docs/blob/master/docs/core/rid-catalog.md
* cake depedency market as private asset and updated to 0.34.1.
* cake.build script improvments
* unit test packages update.
* Since Electron.NET releases its first stable version https://github.com/ElectronNET/Electron.NET/releases/tag/5.22.14 this the first stable version of Cake.Electron.Net
Blind-Striker added a commit to Blind-Striker/Cake.Electron.Net that referenced this pull request Sep 7, 2019
* New electronize add command support added
* Some rid removed and added based on new .NET Core RID catalog. See dotnet/docs#8710 and https://github.com/dotnet/docs/blob/master/docs/core/rid-catalog.md
* cake depedency market as private asset and updated to 0.34.1.
* cake.build script improvments
* unit test packages update.
* Since Electron.NET releases its first stable version https://github.com/ElectronNET/Electron.NET/releases/tag/5.22.14 this the first stable version of Cake.Electron.Net
Blind-Striker added a commit to Blind-Striker/Cake.Electron.Net that referenced this pull request Sep 7, 2019
* New electronize add command support added
* Some rid removed and added based on new .NET Core RID catalog. See dotnet/docs#8710 and https://github.com/dotnet/docs/blob/master/docs/core/rid-catalog.md
* cake depedency market as private asset and updated to 0.34.1.
* cake.build script improvments
* unit test packages update.
* Since Electron.NET releases its first stable version https://github.com/ElectronNET/Electron.NET/releases/tag/5.22.14 this the first stable version of Cake.Electron.Net
* BuildId replaced with $(Rev)
Blind-Striker added a commit to Blind-Striker/Cake.Electron.Net that referenced this pull request Sep 7, 2019
* New electronize add command support added
* Some rid removed and added based on new .NET Core RID catalog. See dotnet/docs#8710 and https://github.com/dotnet/docs/blob/master/docs/core/rid-catalog.md
* cake depedency market as private asset and updated to 0.34.1.
* cake.build script improvments
* unit test packages update.
* Since Electron.NET releases its first stable version https://github.com/ElectronNET/Electron.NET/releases/tag/5.22.14 this the first stable version of Cake.Electron.Net
* BuildId replaced with $(Rev)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants