Skip to content

Add initial commit for package validation #16992

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
Merged
merged 17 commits into from
May 1, 2021
Merged

Add initial commit for package validation #16992

merged 17 commits into from
May 1, 2021

Conversation

Anipik
Copy link
Contributor

@Anipik Anipik commented Apr 17, 2021

The initial commit contains

  • content model for package and validators.
  • targets for running it after the pack task.
  • integrating the api compat.

Things left

  • adding more tests for different validators and with api compat
  • add more validators.
  • add support for baselining the errors

@ghost
Copy link

ghost commented Apr 17, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

eerhardt
eerhardt reviewed Apr 19, 2021
View reviewed changes
src/Compatibility/Microsoft.DotNet.PackageValidation/Microsoft.DotNet.PackageValidation.csproj Outdated
</Reference>
</ItemGroup>

<Target Name="_AddBuildOutputToPackageCore" DependsOnTargets="Publish" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
Copy link
Member

Choose a reason for hiding this comment

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

(for my information) Why don't we need to Publish for net472?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

net472 doesnt create a publish folder on using dotnet pack.

eerhardt
eerhardt reviewed Apr 19, 2021
View reviewed changes
eerhardt
eerhardt reviewed Apr 19, 2021
View reviewed changes
safern
safern reviewed Apr 19, 2021
View reviewed changes
ViktorHofer
ViktorHofer reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
src/Compatibility/Microsoft.DotNet.PackageValidation/ApiCompatRunner.cs
using (Stream leftAssemblyStream = GetFileStreamFromPackage(apicompatTuples.leftAssemblyPackagePath, apicompatTuples.leftAssemblyRelativePath))
using (Stream rightAssemblyStream = GetFileStreamFromPackage(apicompatTuples.rightAssemblyPackagePath, apicompatTuples.rightAssemblyRelativePath))
{
IAssemblySymbol leftSymbols = new AssemblySymbolLoader().LoadAssembly(apicompatTuples.assemblyName, leftAssemblyStream);
Copy link
Member

Choose a reason for hiding this comment

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

All this logic will need to be changed with: #17295

safern
safern reviewed Apr 30, 2021
View reviewed changes
src/Compatibility/Microsoft.DotNet.PackageValidation/GetLastStablePackage.cs
}

// If no suitable package was found, error out.
// TODO: Localize error message and add more diagnostics.
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth to log issues for these TODOs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i will create those

safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern reviewed Apr 30, 2021
View reviewed changes
safern
safern approved these changes May 1, 2021
View reviewed changes
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.

Other than my last comment this looks great! Great work! Excited to see this coming together, and let's open the follow up issues we discussed offline.

@Anipik Anipik merged commit d22bb74 into dotnet:main May 1, 2021
@Anipik Anipik deleted the packageValidation branch May 1, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkolev92 nkolev92 nkolev92 left review comments

@ViktorHofer ViktorHofer ViktorHofer left review comments

@eerhardt eerhardt eerhardt left review comments

@ericstj ericstj ericstj left review comments

@joperezr joperezr joperezr left review comments

@safern safern safern approved these changes

Assignees
No one assigned
Labels
Area-ApiCompat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Anipik @ViktorHofer @safern @nkolev92 @eerhardt @ericstj @joperezr