Skip to content

Native aot stj8#640

Merged
gregsdennis merged 5 commits into
native-aotfrom
native-aot-stj8
Jan 30, 2024
Merged

Native aot stj8#640
gregsdennis merged 5 commits into
native-aotfrom
native-aot-stj8

Conversation

@gregsdennis

Copy link
Copy Markdown
Collaborator

Still has issues with double in netcoreapp3.1. Not sure what to do there.

Also supposedly we can stop targeting net8 directly and just use netstandard 2. I'll try that.

@github-actions

github-actions Bot commented Jan 30, 2024

Copy link
Copy Markdown

Test Results

    15 files  ±0      15 suites  ±0   14s ⏱️ +2s
20 477 tests +5  19 113 ✅ +5  1 364 💤 ±0  0 ❌ ±0 
20 569 runs  +5  19 202 ✅ +5  1 367 💤 ±0  0 ❌ ±0 

Results for commit 2b6188b. ± Comparison against base commit 2ded6ab.

♻️ This comment has been updated with latest results.

@gregsdennis gregsdennis marked this pull request as ready for review January 30, 2024 02:40
@gregsdennis

Copy link
Copy Markdown
Collaborator Author

I think I'm going to keep the multitarget in the project files. There are still a few things (like Enum.GetValues<T>()) that require it. Also, we get the compiler warnings only when devving in .net 8.

@jevansaks

Copy link
Copy Markdown
Contributor

Multi-targeting also enables us to run tests reflection enabled and disabled, so that's also handy.

Comment thread Json.More/Json.More.csproj
Comment thread JsonLogic/JsonWriterExtensions.cs Outdated
@jevansaks

Copy link
Copy Markdown
Contributor

Having done this a bunch these last few days, I prefer the in-source suppressions are better for reviewing/maintaining the code.


Refers to: JsonSchema.Generation/GlobalSuppressions.cs:18 in fb79d00. [](commit_id = fb79d00, deletion_comment = False)

@gregsdennis

Copy link
Copy Markdown
Collaborator Author

I prefer the in-source suppressions are better for reviewing/maintaining the code.

I think this'll go in another PR.

@gregsdennis gregsdennis merged commit e30f440 into native-aot Jan 30, 2024
@gregsdennis gregsdennis deleted the native-aot-stj8 branch January 30, 2024 06:37
#pragma warning disable IL2026, IL3050
// using the options.Write() extension here for some reason causes a StackOverflow
JsonSerializer.Serialize(writer, rule, rule.GetType(), LogicSerializerContext.OptionsManager.SerializerOptions);
options.Write(writer, rule, rule.GetType());

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.

FYI still need to use Suppression attributes instead of #pragma warning disable because library consumers don't see the #pragma warning and the callers will get AOT warnings on our code. The AOT analyzer only sees attributes so we need to suppress that way. See #636

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added it in #638

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants