Uh oh!
There was an error while loading. Please reload this page.
[TrimmableTypeMap] Add typed logger interface - #11095
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors TrimmableTypeMap logging by replacing the Action<string> callback with a typed ITrimmableTypeMapLogger interface, enabling structured/typed log events for the typemap generator pipeline.
Changes:
- Introduces
ITrimmableTypeMapLoggerwith typed log methods for key generator events. - Updates
TrimmableTypeMapGeneratorto accept an optionalITrimmableTypeMapLogger. - Adds MSBuild and test adapters (
MSBuildTrimmableTypeMapLogger,TestTrimmableTypeMapLogger).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Replaces string callback logger with a typed test logger implementation. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Adds an MSBuild logger adapter implementing the new interface and wires it into the task. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Switches generator logging from Action<string> to ITrimmableTypeMapLogger? calls. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Adds the new typed logging interface. |
| readonly ITrimmableTypeMapLogger? logger; | ||
| public TrimmableTypeMapGenerator (Action<string> log) | ||
| public TrimmableTypeMapGenerator (ITrimmableTypeMapLogger? logger = null) | ||
| { | ||
| this.log = log ?? throw new ArgumentNullException (nameof (log)); | ||
| this.logger = logger; |
There was a problem hiding this comment.
The PR description says “No behavioral changes — pure refactoring”, but the TrimmableTypeMapGenerator constructor now accepts a null logger (and even defaults to null) instead of throwing on null as it did with the previous required Action parameter. This changes the API contract and runtime behavior for callers that pass null (they’ll now silently get no logging). If the intent is truly no behavior change, keep the parameter required/non-null and continue throwing on null; otherwise, update the PR description to reflect the contract change.
7443456 to
0342566CompareReplace Action<string> log callback with ITrimmableTypeMapLogger interface in TrimmableTypeMapGenerator. This enables structured logging with typed methods instead of free-form string messages, making it easier to extend with new log categories (e.g., warnings) in follow-up PRs. No behavioral changes — pure refactoring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
0342566 to
d68d7dfCompareUh oh!
There was an error while loading. Please reload this page.
Summary
Replace
Action<string> logcallback withITrimmableTypeMapLoggerinterface inTrimmableTypeMapGenerator. This enables structured logging with typed methods instead of free-form string messages, making it easier to extend with new log categories (e.g., warnings) in follow-up PRs.No behavioral changes — pure refactoring.
Changes
ITrimmableTypeMapLogger— new interface with 8 typed log methodsTrimmableTypeMapGenerator— acceptITrimmableTypeMapLogger?instead ofAction<string>GenerateTrimmableTypeMap— addMSBuildTrimmableTypeMapLoggeradapter classTestTrimmableTypeMapLoggertest double