Skip to content

Clean up formatting and naming#29

Open
keyboardsurfer wants to merge 1 commit into
mainfrom
bw/spotlessApply
Open

Clean up formatting and naming#29
keyboardsurfer wants to merge 1 commit into
mainfrom
bw/spotlessApply

Conversation

@keyboardsurfer

Copy link
Copy Markdown
Member
  • Apply missing spotless changes

- Apply missing spotless changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines 113 to 115
if (!hasFineLocation && !hasCoarseLocation) {
throw IllegalStateException("Location permission is not granted")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

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.

Suggested change
if (!hasFineLocation && !hasCoarseLocation) {
throw IllegalStateException("Location permission is not granted")
}
if (!hasFineLocation && !hasCoarseLocation) {
throw androidx.appfunctions.AppFunctionPermissionRequiredException("Location permission is not granted")
}

Comment on lines +51 to +57
suspend fun geocodeAddress(address: String): LatLng? {
val context = this
if (!Geocoder.isPresent()) {
return null
}

val geocoder = Geocoder(this)
val geocoder = Geocoder(context)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
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)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants