[Mono.Android] Enumify API-37.0, API-37.0 is "Stable"#11151
[Mono.Android] Enumify API-37.0, API-37.0 is "Stable"#11151jonpryor wants to merge 1 commit intodotnet:mainfrom
Conversation
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
Found 4 issues in the API 37 enumification map.csv entries:
- ❌ Wrong enum type:
AlcoholConsumptionRecord.RECORD_TEMPORAL_TYPE_*constants are mapped toAlcoholConsumptionBeverageTypeinstead of a separate temporal-type enum (map.csv:3391-3393) - ❌ Typo:
KillExcessivCpuUsage→KillExcessiveCpuUsage(missing 'e') — this becomes a public API name (map.csv:9754) ⚠️ Namespace casing:Android.Content.Pm.Webappshould beAndroid.Content.PM.Webappto match existingandroid.content.pmconvention (map.csv:2208-2223)⚠️ Redundant prefixes: Several new member names include the enum type name as a prefix, inconsistent with existing members in the same enum types (6 instances acrossNetCapability,DocumentContractFlags,DocumentRootFlags,ProfilingTriggerType)
👍 The Documentation path updates (xamarin-android → dotnet/android) and metadata ns-replace additions look correct. The LIGHT_CAPABILITY_* flags fix and CarrierConfigManager ? → I decisions are good cleanup.
Review generated by android-reviewer from review guidelines.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #11151
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - #11151
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Android PR Reviewer for issue #11151 · ● 26.6M
| E,37,android/health/connect/datatypes/AlcoholConsumptionRecord.RECORD_TEMPORAL_TYPE_INSTANT,0,Android.Health.Connect.Datatypes.AlcoholConsumptionBeverageType,RecordTemporalTypeInstant,remove, |
There was a problem hiding this comment.
🤖 ❌ Bugs & correctness — RECORD_TEMPORAL_TYPE_* constants are mapped to the wrong enum type AlcoholConsumptionBeverageType. These have nothing to do with beverage types — they describe temporal record types (Instant, Interval, LocalDate).
Compare with SymptomRecord.RECORD_TEMPORAL_TYPE_* which correctly uses a separate SymptomRecordTemporalType enum. These should use something like AlcoholConsumptionRecordTemporalType instead.
-E,37,...AlcoholConsumptionRecord.RECORD_TEMPORAL_TYPE_INSTANT,0,Android.Health.Connect.Datatypes.AlcoholConsumptionBeverageType,RecordTemporalTypeInstant,remove,
+E,37,...AlcoholConsumptionRecord.RECORD_TEMPORAL_TYPE_INSTANT,0,Android.Health.Connect.Datatypes.AlcoholConsumptionRecordTemporalType,Instant,remove,
(same for RECORD_TEMPORAL_TYPE_INTERVAL and RECORD_TEMPORAL_TYPE_LOCAL_DATE on lines 3392-3393)
| @@ -6646,6 +6967,7 @@ E,28,android/net/NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED,21,Android.Net | |||
| E,21,android/net/NetworkCapabilities.NET_CAPABILITY_NOT_VPN,15,Android.Net.NetCapability,NotVpn,remove, | |||
| E,33,android/net/NetworkCapabilities.NET_CAPABILITY_PRIORITIZE_BANDWIDTH,35,Android.Net.NetCapability,PrioritizeBandwidth,remove, | |||
| E,33,android/net/NetworkCapabilities.NET_CAPABILITY_PRIORITIZE_LATENCY,34,Android.Net.NetCapability,PrioritizeLatency,remove, | |||
| E,37,android/net/NetworkCapabilities.NET_CAPABILITY_PRIORITIZE_UNIFIED_COMMUNICATIONS,38,Android.Net.NetCapability,NetCapabilityPrioritizeUnifiedCommunications,remove, | |||
There was a problem hiding this comment.
🤖 NetCapabilityPrioritizeUnifiedCommunications has a redundant NetCapability prefix. Existing members in the same enum use clean names: PrioritizeBandwidth, PrioritizeLatency, CaptivePortal, etc. This should be PrioritizeUnifiedCommunications for consistency.
Also affected (same pattern of redundant Flag/TriggerType prefixes inconsistent with existing members in the same enum type):
FlagSupportsRestore→SupportsRestoreinDocumentContractFlags(line 10051)FlagSupportsTrash→SupportsTrashinDocumentContractFlags(line 10054)FlagLimitedFunctionalityWhenOffline→LimitedFunctionalityWhenOfflineinDocumentRootFlags(line 10065)FlagSupportsQueryTrash→SupportsQueryTrashinDocumentRootFlags(line 10070)TriggerTypeOom→OominProfilingTriggerType(line 9756)
| @@ -2117,6 +2205,22 @@ E,36,android/content/pm/SigningInfo.VERSION_SIGNING_BLOCK_V4,4,Android.Content.P | |||
| E,31,android/content/pm/verify/domain/DomainVerificationUserState.DOMAIN_STATE_NONE,0,Android.Content.PM.Verify.Domain.DomainState,None,remove, | |||
| E,31,android/content/pm/verify/domain/DomainVerificationUserState.DOMAIN_STATE_SELECTED,1,Android.Content.PM.Verify.Domain.DomainState,Selected,remove, | |||
| E,31,android/content/pm/verify/domain/DomainVerificationUserState.DOMAIN_STATE_VERIFIED,2,Android.Content.PM.Verify.Domain.DomainState,Verified,remove, | |||
| E,37,android/content/pm/webapp/WebAppInstallRequest.RESULT_CANCELLED_BY_USER,6,Android.Content.Pm.Webapp.WebAppInstallResultCode,CancelledByUser,remove, | |||
There was a problem hiding this comment.
🤖 Android.Content.Pm.Webapp but all existing android.content.pm entries use Android.Content.PM (uppercase PM). This should be Android.Content.PM.Webapp for consistency.
All 16 entries in this block (lines 2208-2223) are affected.
| E,36,android/os/ProfilingTrigger.TRIGGER_TYPE_APP_FULLY_DRAWN,1,Android.OS.ProfilingTriggerType,AppFullyDrawn,remove, | ||
| E,36.1,android/os/ProfilingTrigger.TRIGGER_TYPE_APP_REQUEST_RUNNING_TRACE,3,Android.OS.ProfilingTriggerType,TriggerTypeAppRequestRunningTrace,remove, | ||
| E,37,android/os/ProfilingTrigger.TRIGGER_TYPE_COLD_START,10,Android.OS.ProfilingTriggerType,ColdStart,remove, | ||
| E,37,android/os/ProfilingTrigger.TRIGGER_TYPE_KILL_EXCESSIVE_CPU_USAGE,9,Android.OS.ProfilingTriggerType,KillExcessivCpuUsage,remove, |
There was a problem hiding this comment.
🤖 ❌ Bugs & correctness — Typo in member name: KillExcessivCpuUsage is missing the e — should be KillExcessiveCpuUsage. This becomes a public API name, so it will be permanent once shipped.
-...,KillExcessivCpuUsage,remove,
+...,KillExcessiveCpuUsage,remove,
Rule: Typos in user-visible strings (Postmortem #61)
Context: http://github.com/jpobst/BindingStudio Context: dotnet#10005 Use jpobst/BindingStudio to begin enumifying API-36.1. Note: current dotnet/java-interop emits an "extra" `,` on `map.csv` output, which would make for a "noisy" diff (every line changed!). The diff size is reduced by removing trailing commas: sed 's/,$//' < src/Mono.Android/new-map.csv > src/Mono.Android/map.csv This keeps the diff to a reasonable size. Sanity check: during enumification, this should have no matches: git grep '\.[A-Z][a-z]\.' src/Mono.Android/map.csv Two-letter namespace parts should be all upper-case. TODO (lol): update `map.csv` to current BindingStudio output after this is merged. WIP. Current state: finished **Creating enums**. Need to perform: * **Extract methods that possibly need enums** * **Mapping methods** Commit [Extract constants from API][0] output. [0]: https://github.com/dotnet/android/blob/e67a3fd36ca5e24b0007a170ea77037e4437f9d3/Documentation/workflow/HowToAddNewApiLevel.md#extract-constants-from-api
8de222f to
54877b0
Compare
Context: http://github.com/jpobst/BindingStudio
Context: #10005
Use jpobst/BindingStudio to begin enumifying API-36.1.
Note: current dotnet/java-interop emits an "extra"
,onmap.csvoutput, which would make for a "noisy" diff (every line changed!). The diff size is reduced by removing trailing commas:This keeps the diff to a reasonable size.
TODO (lol): update
map.csvto current BindingStudio output after this is merged.WIP.
Current state: finished Creating enums.
Need to perform:
Commit Extract constants from API output.
Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include: