Clean up formatting and naming#29
Conversation
keyboardsurfer
commented
Jul 14, 2026
- Apply missing spotless changes
- Apply missing spotless changes
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on code formatting, updating the KSP version to 2.3.9, increasing the minSdk to 36, and refactoring AppFunction services by renaming base classes to abstract classes and removing @RequiresApi(36) annotations. Feedback on the changes suggests throwing a more specific AppFunctionPermissionRequiredException instead of IllegalStateException when location permissions are missing, and removing a redundant local context variable in AbstractBuiltInAppFunctions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (!hasFineLocation && !hasCoarseLocation) { | ||
| throw IllegalStateException("Location permission is not granted") | ||
| } |
There was a problem hiding this comment.
Throwing a generic IllegalStateException when location permissions are missing prevents the AppFunction framework and the LLM agent from handling the error gracefully.
Instead, you should throw androidx.appfunctions.AppFunctionPermissionRequiredException. This allows the framework to recognize the missing permission and potentially prompt the user or return a structured error to the assistant.
| if (!hasFineLocation && !hasCoarseLocation) { | |
| throw IllegalStateException("Location permission is not granted") | |
| } | |
| if (!hasFineLocation && !hasCoarseLocation) { | |
| throw androidx.appfunctions.AppFunctionPermissionRequiredException("Location permission is not granted") | |
| } |
| suspend fun geocodeAddress(address: String): LatLng? { | ||
| val context = this | ||
| if (!Geocoder.isPresent()) { | ||
| return null | ||
| } | ||
|
|
||
| val geocoder = Geocoder(this) | ||
| val geocoder = Geocoder(context) |
There was a problem hiding this comment.
The local variable context is redundant here. Since AbstractBuiltInAppFunctions extends AppFunctionService (which is a Context), this can be passed directly to the Geocoder constructor. Removing the intermediate context variable simplifies the code and reduces boilerplate.
| suspend fun geocodeAddress(address: String): LatLng? { | |
| val context = this | |
| if (!Geocoder.isPresent()) { | |
| return null | |
| } | |
| val geocoder = Geocoder(this) | |
| val geocoder = Geocoder(context) | |
| suspend fun geocodeAddress(address: String): LatLng? { | |
| if (!Geocoder.isPresent()) { | |
| return null | |
| } | |
| val geocoder = Geocoder(this) |