Skip to content

testing net8#39410

Closed
TimothyMothra wants to merge 2 commits intomainfrom
tilee/aot
Closed

testing net8#39410
TimothyMothra wants to merge 2 commits intomainfrom
tilee/aot

Conversation

@TimothyMothra
Copy link

@TimothyMothra TimothyMothra commented Oct 19, 2023

I modified the AOT Compatibility Test App to run on NET 8.

We still have 16 outstanding warnings.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.


Write-Host "Actual warning count is:", $actualWarningCount
$expectedWarningCount = 7
$expectedWarningCount = 16
Copy link
Member

Choose a reason for hiding this comment

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

The 4 from the JsonConverters are the only that are expected

Copy link
Author

Choose a reason for hiding this comment

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

After taking your changes, I can confirm this is down to 4. All are related to JsonConverters. :)
Is there a plan for these outstanding warnings?

Copy link
Member

@m-redding m-redding Oct 27, 2023

Choose a reason for hiding this comment

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

The outstanding warnings have to be baselined in tests, I can link the discussion that led to this decision if you're interested in more information, but to briefly summarize, DynamicData and MutableJsonDocument are not AOT compatible at all partially because of the custom converters. If we suppress the warnings on the converters and only annotate the actual class, there are ways that non-AOT compatible code could get missed by the trimmer (it's unlikely - but still possible). So, to be safe, we opted to leave the warnings on these classes.

Since the test uses the <TrimmerRootAssembly> tag, the trimmer is very liberal in the code it considers part of this assembly. Because of this, DynamicData and MutableJsonDocument get pulled in because there is a constructor in RequestContent that has a DynamicData parameter, even though (I think) the constructor-in-question is not actually used anywhere in your library. This means that as long as your library isn't using DynamicData anywhere, a customer using Azure.Monitor.OpenTelemetry.Exporter won't see these warnings on their end.

I hope I explained this okay, but happy to answer other questions if needed.

@github-actions
Copy link

Hi @TimothyMothra. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@github-actions github-actions bot added the no-recent-activity There has been no recent activity on this issue. label Dec 29, 2023
@github-actions
Copy link

github-actions bot commented Jan 5, 2024

Hi @TimothyMothra. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing /reopen if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the no-recent-activity label; otherwise, this is likely to be closed again with the next cleanup pass.

@github-actions github-actions bot closed this Jan 5, 2024
@jsquire jsquire deleted the tilee/aot branch February 18, 2025 22:46
haiyuazhang pushed a commit to haiyuazhang/azure-sdk-for-net that referenced this pull request Mar 8, 2026
* Copy files from stable/2025-03-01

Copied the files in a separate commit.
This allows reviewers to easily diff subsequent changes against the previous spec.

* Update version to stable/2025-05-01

Updated the API version from stable/2025-03-01 to stable/2025-05-01.

* Added tag for 2025-05-01 in readme file

* Update Suppression and directives from previous verison

* Update version to stable 2025-05-01 (Azure#39294)

Co-authored-by: Santosh Lokarapu <v-slokar@microsoft.com>

* Service Gateway Swagger changes (Azure#39221)

* Sgw change

* ModelFix

* Model issue fix

* Fix

* Model fix

* Fixed comment

* Updated description for zone

* Azure Firewall Policy: Add New IDPS Profiles (Azure#36855)

* Update IDPS Profile names

* Add examples for IDPS profile

* Update Firewall Policy PUT example IDPS profile

* Update profile name and enum order

* Remove delete example

* Update profile names to emerging, extended and core

* Add Off as default profile

* Move changes to latest release

* Revert PUT example change on older version

* Move changes to release 2025-05-01

* Update examples release version

* Update profile order

* Update profiles description

* Add vna top level resource 2025-05-01 (Azure#39341)

* Add a new top level resource for VNA

* add suppression for a known issue.

* add 1 more suppression due to known issue

* added x-ms-parameter-location as per go sdk team suggestion

* one more place to add for x-ms-parameter-location attribute

---------

Co-authored-by: Prajjwal Kamboj <kamboj.prjwl@gmail.com>

* Update the AppGW WAF spec with CAPTCHA action and Cookie expiary (Azure#39362)

* update the spec with CAPTCHA action and cookies expiary

* whitespace issue

* Fix description for CAPTCHA cookie expiration

* Lint error fix (Azure#39410)

* Lint error fix

* Response Change

---------

Co-authored-by: Prajjwal Kamboj <pkamboj@microsoft.com>
Co-authored-by: santoshgh317 <santoshgh317@gmail.com>
Co-authored-by: Santosh Lokarapu <v-slokar@microsoft.com>
Co-authored-by: anjbal <137822143+anjbal@users.noreply.github.com>
Co-authored-by: Saloni Rajeev Kumar <salonirk11@gmail.com>
Co-authored-by: Arjun Patel <94936045+arjun-d-patel@users.noreply.github.com>
Co-authored-by: Yaniv Sagron <yanivsag@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core Monitor - Exporter Monitor OpenTelemetry Exporter no-recent-activity There has been no recent activity on this issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants