Skip to content

Conversation

jaredpar
Copy link
Member

This moves us to the latest version of our test reference assemblies. There is a bit of churn here because the API changed to be more efficient about how it loaded data.

There are a series of commits here:

  • One commit for every NuPkg that I upgraded so you can see the impact individually.
  • One commit where I fixed the style of TargetFrameworkUtil to be more readable. It was hard to maintain the invariants around TestBase lazy loading when all of the references were so far off the screen.

@jaredpar jaredpar requested review from a team as code owners April 29, 2024 18:46
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 29, 2024
@jaredpar
Copy link
Member Author

@dotnet/roslyn-compiler PTAL

@jaredpar jaredpar added this to the 17.11 milestone Apr 29, 2024
NetStandard20.mscorlib,
NetStandard20.netstandard,
NetStandard20.SystemRuntime,
NetStandard20.References.mscorlib,
Copy link
Member

Choose a reason for hiding this comment

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

nit: was looking at the code structure in the basic-reference-assemblies repo (for example), consider putting some xml doc comments on References and ReferenceInfos.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah looking at that code isn't going to be particularly useful. It's 99% just generated code.

XML docs are a good idea though. Will add those

Copy link
Member Author

Choose a reason for hiding this comment

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

jaredpar/basic-reference-assemblies@5eeecbf

Those docs will show up next upgrade of these packages

public static ImmutableArray<MetadataReference> DefaultVbReferences => ImmutableArray.Create<MetadataReference>(Net451.mscorlib, Net451.System, Net451.SystemCore, Net451.MicrosoftVisualBasic);
public static ImmutableArray<MetadataReference> MinimalReferences => ImmutableArray.Create(TestBase.MinCorlibRef);
public static ImmutableArray<MetadataReference> MinimalAsyncReferences => ImmutableArray.Create(TestBase.MinAsyncCorlibRef);
public static ImmutableArray<MetadataReference> Mscorlib45ExtendedReferences => ImmutableArray.Create<MetadataReference>(Net451.mscorlib, Net451.System, Net451.SystemCore, TestBase.ValueTupleRef, Net451.SystemRuntime);
Copy link
Member

Choose a reason for hiding this comment

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

Could we maintain the original order? That would make the diff more readable (easier to review)

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the order specifically to line up with the comment. That comment is warning on the dangers of eagerly digging into TestBase. references as they are Lazy<T> based so we're kinda defeating things by eagerly digging in. This change groups all of the bundles that use TestBase. into a group, put them directly below the warning comment and use => to fulfill the "on demand" nature. All the ones in the second group do not depend on TestBase. and can be initialized more efficiently.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 8)

@AlekseyTs
Copy link
Contributor

This moves us to the latest version of our test reference assemblies.

Perhaps I am misinterpreting the purpose of this change, but it doesn't look like we are adding ref assemblies for .Net 9.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

I do not see anything obviously wrong :-) (Commit 8)

public static ImmutableArray<MetadataReference> MinimalReferences => ImmutableArray.Create(TestBase.MinCorlibRef);
public static ImmutableArray<MetadataReference> MinimalAsyncReferences => ImmutableArray.Create(TestBase.MinAsyncCorlibRef);
public static ImmutableArray<MetadataReference> Mscorlib45ExtendedReferences => ImmutableArray.Create<MetadataReference>(Net451.mscorlib, Net451.System, Net451.SystemCore, TestBase.ValueTupleRef, Net451.SystemRuntime);
public static ImmutableArray<MetadataReference> Mscorlib46ExtendedReferences => ImmutableArray.Create<MetadataReference>(Net461.References.mscorlib, Net461.References.System, Net461.References.SystemCore, TestBase.ValueTupleRef, Net461.References.SystemRuntime);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: why are these not collection expressions like the others?

Copy link
Member Author

Choose a reason for hiding this comment

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

Cause I forgot to go back and change these 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants