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

Draft of adding an AOT compatibility CI Pipeline #38634

Closed
wants to merge 4 commits into from

Conversation

m-redding
Copy link
Member

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.


<ItemGroup>
<!-- Update this dependency to its latest, which has all the annotations -->
<PackageReference Include="Microsoft.Extensions.Logging.Configuration" Version="8.0.0-preview.7.23375.6" />
Copy link
Member

Choose a reason for hiding this comment

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

If I check in #38500 you should remove the Version attribute.


<ItemGroup>
<ProjectReference Include="..\..\..\sdk\$ServiceDirectory\$PackageName\src\$PackageName.csproj" />
<TrimmerRootAssembly Include="$PackageName" />
Copy link
Member

Choose a reason for hiding this comment

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

Nit: spacing is off.

}
}

Write-Host "Checking against expected number of warnings"
Copy link
Member

Choose a reason for hiding this comment

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

This seems very fragile. Can we instead check against an array of patterns e.g.,

$patterns = @(
  'IL2026.*Azure.Core.RequestContentHelper.FromEnumerable\(IEnumerable`1<BinaryData>\)'
  'IL3050.*Azure.Core.RequestContentHelper.FromEnumerable\(IEnumerable`1<BinaryData>\)'
  # ...
)
$output = `dotnet publish` # and any other args
$warnings = $output -split "`n" | select-string -pattern 'IL\d+' | select-string -pattern $patterns -notmatch
if ($warnings.Count -gt 0) {
  Write-Host "Found additional warnings:`n$warnings"
  exit $warnings.Count
}

@m-redding m-redding changed the title [WIP] Add an opt-in AOT compatibility CI Pipeline Draft of adding an AOT compatibility CI Pipeline Oct 25, 2023
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net7.0</TargetFramework>
Copy link
Contributor

Choose a reason for hiding this comment

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

Per Vitek, this should target net8.0 #39428 (comment)

Suggested change
<TargetFramework>net7.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>

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'm working on this in a separate branch because I ended up needing to change the approach a bit, but we can't target net 8 in the CI yet :(

@m-redding m-redding closed this Dec 11, 2023
@m-redding m-redding deleted the add-aot-pipeline branch January 10, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants