Skip to content

use packages instead of package report for packageInstallation and closure verification#49315

Merged
Anipik merged 14 commits intodotnet:mainfrom
Anipik:install
Mar 22, 2021
Merged

use packages instead of package report for packageInstallation and closure verification#49315
Anipik merged 14 commits intodotnet:mainfrom
Anipik:install

Conversation

@Anipik
Copy link
Contributor

@Anipik Anipik commented Mar 8, 2021

  • use microsoft.dotnet.packagevalidation package to get the list of frameworks where a package should be installed and tested.

This eliminated one of the use of the package reports which we intend to remove in the new packaging system.
This also eliminates a lot of redundant test frameworks.

Arcade PR dotnet/arcade#7057

@ghost
Copy link

ghost commented Mar 8, 2021

Tagging subscribers to this area: @safern, @ViktorHofer, @Anipik
See info in area-owners.md if you want to be subscribed.

Issue Details
  • use microsoft.dotnet.packagevalidation package to get the list of frameworks where a package should be installed and tested.

This eliminated one of the use of the package reports which we intend to remove in the new packaging system.
This also eliminates a lot of redundant test frameworks.

Arcade PR dotnet/arcade#7057

Author: Anipik
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@Anipik Anipik requested review from ericstj, joperezr and safern March 8, 2021 18:36
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

Besides my comments I can't add much value as I don't know about the package report system. Hopefully @joperezr or @ericstj can provide a more in depth review.

@Anipik
Copy link
Contributor Author

Anipik commented Mar 18, 2021

@safern @ViktorHofer @ericstj can you take another look here ?

<ExcludePackages Include="Microsoft.AspNetCore.Internal.Transport" />
<ExcludePackages Include="$(ExcludePackages)" />

<PackageReports Condition="'@(PackagesToTest)' == ''" Include="$(PackageReportDir)*.json" Exclude="@(ExcludePackages->'$(PackageReportDir)%(Identity).json')"/>
Copy link
Member

Choose a reason for hiding this comment

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

Presumably you'll clean this up later when we eliminate the harvesting and no longer need the harvesting check below?

Copy link
Contributor Author

@Anipik Anipik Mar 19, 2021

Choose a reason for hiding this comment

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

yes correct, hopefully we can clean up the entire reports related code after that.

@Anipik
Copy link
Contributor Author

Anipik commented Mar 21, 2021

@ViktorHofer @ericstj i addressed the major version issue with the regex. can u take a look ?

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@Anipik Anipik merged commit 6f73cce into dotnet:main Mar 22, 2021
@Anipik Anipik deleted the install branch March 22, 2021 17:59
<Copy SourceFiles="@(TestSupportFiles)" DestinationFolder="%(TestSupportFiles.DestinationFolder)" />
</Target>

<UsingTask TaskName="GetCompatiblePackageTargetFrameworks" AssemblyFile="$(DotNetPackageValidationAssembly)"/>
Copy link
Member

Choose a reason for hiding this comment

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

Curious, where does this project PackageReference the package validation package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about workflow, but all the packageReferences in tools.props are imported here

@ghost ghost locked as resolved and limited conversation to collaborators Apr 22, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants