[AOT compatible] Clean up some AOT build issue in PowerAccent.Core #36264
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
crutkas
added
Needs-Review
jaimecbernardo
added
the
Don't merge - Hold for release
|
Hi, we've added the "Don't Merge" tag to the PR, since we're trying to keep the repo stable for the release and possible hotfix if necessary. Please don't merge the PR while the tag is still in here. This allows people to review the PR and approve with less fear that it'll get merged 😄 |
jaimecbernardo
removed
the
Don't merge - Hold for release
moooyo
requested review from
crutkas,
snickler,
jaimecbernardo and
yeelam-gordon
| { | ||
| [JsonSourceGenerationOptions(WriteIndented = true)] | ||
| [JsonSerializable(typeof(SettingsService))] | ||
| public partial class SourceGenerationContext : JsonSerializerContext |
There was a problem hiding this comment.
I believe we should use a common Helpers or Contexts folder to put these JsonSerializerContext classes underneath, as there may ones that encompass multiple JsonSerializable types.
There was a problem hiding this comment.
@moooyo - Other than this, change, it LGTM.
There was a problem hiding this comment.
Agree. any suggestion? How about move to ManagedCommon? @snickler
There was a problem hiding this comment.
Just one confuse, It seems that we cannot centrailize it? Let me explain why I think that.
Consider we have a sub-project named JsonContextCommon, which include all the jsonSerializerContext we need.
Then, if we add a new jsonSerializerContext, we also need to add the [JsonSerializeable(typeof(T))]. Which require us to ref the Type T sub-project in the JsonContextCommon sub-project.
Now, we need to use this context. So, we need to ref JsonContextCommon sub-project in the project which we want to use it. We will necounter cycle reference error.
Only if we move all Type T to a independent sub-project such as ModelCommon, can we move the JsonSerializerContext to the same centralized location.
Summary of the Pull Request
PR Checklist
manual test passed in my local env
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed