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

Rename corefx -> runtime libraries #105285

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

am11
Copy link
Member

@am11 am11 commented Jul 22, 2024

Continues on #105275

@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 Jul 22, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 22, 2024
Copy link
Member Author

Choose a reason for hiding this comment

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

@jakobbotsch, I deleted these scripts because they seem outdated since the repo unification. Please let me know if they should stay.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's fine... I don't think anyone uses or would use this anymore. cc @dotnet/jit-contrib

@am11 am11 added area-Meta and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 22, 2024
Copy link
Contributor

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

@@ -1,1325 +0,0 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

These scripts seem outdated and unused by anything in the repo.

…Services/Unsafe.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
@@ -7,7 +7,7 @@
using System.Collections.Generic;
using System.Threading.Tasks;

namespace CoreFXTestLibrary
Copy link
Member

@jkotas jkotas Jul 27, 2024

Choose a reason for hiding this comment

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

CoreFXTestLibrary was one of the old internal .NET test frameworks used before OSS. The native aot dynamic generics tests are the only place where it is still used. We have migrated away from it everywhere else.

I think RuntimeLibrariesTest is too noble name for it. I would leave it alone. Ideally, all this would be deleted and replaced with xunit (our xunit fork that is not reflection heavy that we use in other places).

If you really want to get rid of the CoreFX name, I would rename it to something that better describes what it is, e.g. LegacyTestLibrary.

@@ -72,7 +72,7 @@ public string CoreRootOutputPath(CompilerIndex index, bool isFramework)
/// <summary>
/// Creates compiler runner instances for each supported compiler based on the populated BuildOptions.
/// </summary>
/// <param name="isFramework">True if compiling the CoreFX framework assemblies</param>
/// <param name="isFramework">True if compiling the runtime libraries assemblies</param>
Copy link
Member

Choose a reason for hiding this comment

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

We've typically used "core libraries" (analogous to corelib which is just one). I've not heard "runtime libraries ". I edited the glossary too Core libraries

Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Meta community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants