Skip to content

Fix analyzer reference in AuditReports package - #5894

Merged
RussKie merged 3 commits into
mainfrom
dletterman/compliance-reports-regression
Feb 14, 2025
Merged

Fix analyzer reference in AuditReports package#5894
RussKie merged 3 commits into
mainfrom
dletterman/compliance-reports-regression

Conversation

@dariusclay

@dariusclaydariusclay commented Feb 13, 2025

Copy link
Copy Markdown
Member

Fix regression due to #5531

It broke the AuditReports extension since it will not be able to resolve the dependency on Microsoft.Gen.MetadataExtractor.

Microsoft Reviewers: Open in CodeFlow

@dariusclay
dariusclay requested review from a team as code ownersFebruary 13, 2025 12:31
Comment thread.config/dotnet-tools.json Outdated
@IbrahimNadaa

IbrahimNadaa commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

Hello ,
I am the one who made that pr if there is anything i can help , I am available

@dariusclay

Copy link
Copy Markdown
MemberAuthor

Hello , I am the one who made that pr if there is anything i can help , I am available

Thanks, I think the implementation is fine, just needed to change ProjectReference -> AnalyzerReference.

@IbrahimNadaa

Copy link
Copy Markdown
Contributor

Hello , I am the one who made that pr if there is anything i can help , I am available

Thanks, I think the implementation is fine, just needed to change ProjectReference -> AnalyzerReference.
i can do it right now and i will run the tests , if you want

@dariusclay

Copy link
Copy Markdown
MemberAuthor

@IbrahimMNada can you pull my branch and run your tests? Changes are already done in this PR.

@IbrahimNadaa

Copy link
Copy Markdown
Contributor

I ran the tests , all seems good, to be 100% sure i can try the package on a sperate fresh project if you'd like

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

‼️Found issues‼️

ProjectCoverage TypeExpectedActual
Microsoft.Extensions.Caching.HybridLine8682.92 🔻
Microsoft.Extensions.AI.Evaluation.QualityLine887.57 🔻
Microsoft.Extensions.AI.Evaluation.QualityBranch8816.42 🔻
Microsoft.Gen.MetadataExtractorLine9857.35 🔻
Microsoft.Gen.MetadataExtractorBranch9862.5 🔻
Microsoft.Extensions.AI.Evaluation.ReportingLine8872.06 🔻
Microsoft.Extensions.AI.Evaluation.ReportingBranch8864.8 🔻
Microsoft.Extensions.AI.Evaluation.ConsoleLine888.26 🔻
Microsoft.Extensions.AI.Evaluation.ConsoleBranch8817.07 🔻
Microsoft.Extensions.AI.OpenAILine7768.75 🔻
Microsoft.Extensions.AI.OpenAIBranch7750.41 🔻
Microsoft.Extensions.AI.OllamaLine8078.2 🔻
Microsoft.Extensions.AI.EvaluationLine8858.67 🔻
Microsoft.Extensions.AI.EvaluationBranch8856.67 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

ProjectExpectedActual
Microsoft.Extensions.AI8889
Microsoft.Extensions.AI.Abstractions8385
Microsoft.Extensions.AI.AzureAIInference9192

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=951242&view=codecoverage-tab

@eduherminio
eduherminio self-requested a review February 13, 2025 13:23
@dariusclay
dariusclay enabled auto-merge (squash) February 13, 2025 13:24

@eduherminioeduherminio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@dariusclay

Copy link
Copy Markdown
MemberAuthor

@RussKie@amadeuszl for approvals

@RussKie

Copy link
Copy Markdown
Contributor

LGTM.

Have you confirmed the change is working as before? E.g., you can build version 9.3 locally, pull dotnet/extensions-samples#645, point it at the locally built version, and verify it can be built.

@RussKieRussKie added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Feb 13, 2025
@IbrahimNadaa

IbrahimNadaa commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

I Tried to run this Microsoft.Extensions.AuditReports package locally on a custom project, I could not find the .target file ....

the analyzers do not fire when i build... is there any documents can help me generate nupkg with the right .target file

@RussKie

Copy link
Copy Markdown
Contributor

I could not find the .target file ....

How did you build and pack? What errors did you observe?

Here's a doc with building instructions. To pack you should run build.cmd -pack to generate nupks under /artifacts folder.
The command would look something like:

.\build.cmd -ci -restore -build -pack /p:OfficialBuildId=20252015.01

@IbrahimNadaa

IbrahimNadaa commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

thanks for pointing out what i was missing , I was able to create a package and run locally on a side project.

this branch contains the same changes as the one main...IbrahimMNada:extensions:meta-data-extractor

I generated a package from it then used on a side project , which is this one
https://github.com/IbrahimMNada/MetaDataExtractorTesting/tree/main/WebApplication1

i defined this in order to test them

image

and thankfully it worked

image

However, i noticed that ComplianceReport creates a directory if it does not exist, while MetricsReports & MetadataReports Do not. i will create an issue related to this

I believe its related to Microsoft.Extensions.Compliance.Testing , I'm not sure if its attended or not.

@dariusclay

Copy link
Copy Markdown
MemberAuthor

@RussKie confirmed Microsoft.Extensions.AuditReports works. Locally packed and updated samples to test.

@dotnet-policy-servicedotnet-policy-serviceBot removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Feb 14, 2025
@RussKie
RussKie merged commit 0c97648 into mainFeb 14, 2025
@RussKie
RussKie deleted the dletterman/compliance-reports-regression branch February 14, 2025 22:40
@RussKie

RussKie commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

Thank you, folks, for jumping in quickly and fixing the issue. Let's monitor the dependency flow now.

Sign up for freeto 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

@dariusclay@IbrahimNadaa@dotnet-comment-bot@RussKie@eduherminio@evgenyfedorov2