-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spotless updates and lint #55
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @cartland, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on improving code formatting, updating library versions, and resolving linting issues in the Fruitties project. The changes are primarily driven by running ./gradlew :androidApp:spotlessApply
and ./gradlew lint
. It addresses formatting inconsistencies, updates library versions in libs.versions.toml
, fixes a duplicate declaration for iOS architectures, disables a ktlint error for Composable functions, and specifies the iOS bundle ID to resolve a compiler warning.
Highlights
- Code Formatting: Applies formatting updates using
./gradlew :androidApp:spotlessApply
to ensure consistent code style. - Dependency Updates: Updates library versions in
libs.versions.toml
to use the latest versions of dependencies. - ktlint Configuration: Configures ktlint to ignore function naming errors for Composable functions, allowing them to start with a capital letter.
- iOS Configuration: Specifies the iOS bundle ID in the project settings to resolve a compiler warning.
- iOS Architecture Fix: Fixes a duplicate declaration of
iossimulatorarm64
instead ofiosarm64
inlibs.versions.toml
.
Changelog
Click here to see the changelog
- Fruitties/.editorconfig
- Adds a ktlint configuration to ignore function naming errors for Composable functions, allowing them to start with a capital letter. Lines 1-4
- Fruitties/androidApp/src/main/java/com/example/fruitties/android/MyApplicationTheme.kt
- Reformatted the code for better readability, specifically around the
colors
,typography
, andshapes
definitions within theMyApplicationTheme
composable function. Lines 38-66
- Reformatted the code for better readability, specifically around the
- Fruitties/androidApp/src/main/java/com/example/fruitties/android/di/App.kt
- Adds an empty line for better readability. Line 26
- Fruitties/androidApp/src/main/java/com/example/fruitties/android/ui/ListScreen.kt
- Reformatted code for better readability, specifically around the
extras
andviewModel
definitions. Lines 78-86 - Reformatted code for better readability in the
CenterAlignedTopAppBar
andcontentWindowInsets
sections. Lines 98-113 - Reformatted code for better readability around the
Spacer
withwindowInsetsBottomHeight
. Lines 152-154 - Reformatted code for better readability in the
FruittieItem
composable, specifically around theCard
and its content. Lines 169-221 - Reformatted code for better readability in the
CartDetailsView
composable. Lines 231-234
- Reformatted code for better readability, specifically around the
- Fruitties/gradle/libs.versions.toml
- Updates the version of
ksp
from2.1.10-1.0.29
to2.1.10-1.0.30
. Line 28 - Fixes a duplicate declaration of
iossimulatorarm64
instead ofiosarm64
by correcting the module name forkotlinx-coroutines-core-iosarm64
. Line 57
- Updates the version of
- Fruitties/iosApp/iosApp.xcodeproj/project.pbxproj
- Specifies the iOS bundle ID (
com.example.fruitties.ios
) in the project settings for both the Debug and Release configurations. Lines 336 and 365
- Specifies the iOS bundle ID (
- Fruitties/iosApp/iosApp/Info.plist
- Moves the
<key>UILaunchScreen</key>
block to be after the<key>UIApplicationSupportsMultipleScenes</key>
block. Lines 28-29 - Removes a duplicate
<key>UILaunchScreen</key>
block at the end of the file. Lines 45-46
- Moves the
- Fruitties/shared/build.gradle.kts
- Removes unused imports
org.jetbrains.kotlin.gradle.ExperimentalKotlinGradlePluginApi
andorg.jetbrains.kotlin.gradle.dsl.JvmTarget
. Lines 16-17 - Adds
binaryOptions["bundleId"] = iosBundleId
to theiosX64
,iosArm64
, andiosSimulatorArm64
configurations to specify the iOS bundle ID. Lines 59, 66, and 73 - Removes an unnecessary empty line. Line 142
- Removes unused imports
- Fruitties/shared/src/androidMain/kotlin/com/example/fruitties/di/Factory.android.kt
- Reformatted code for better readability, specifically around the
Factory
class and its functions. Lines 27-47 - Replaces
dbFileName
withDB_FILE_NAME
. Line 31
- Reformatted code for better readability, specifically around the
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/DataRepository.kt
- Reformatted code for better readability, specifically around the
cartDetails
property and theloadData
function. Lines 37-45 and 60-61
- Reformatted code for better readability, specifically around the
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/database/AppDatabase.kt
- Replaces
dbFileName
withDB_FILE_NAME
. Line 35
- Replaces
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/database/CartDataStore.kt
- Reformatted code for better readability, specifically around the
CartJsonSerializer
object and theCartDataStore
class. Lines 43-71
- Reformatted code for better readability, specifically around the
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/database/FruittieDao.kt
- Removes an unnecessary empty line. Line 28
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/di/Factory.kt
- Reformatted code for better readability, specifically around the
commonCreateApi
function. Lines 34-41
- Reformatted code for better readability, specifically around the
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/network/FruittieApi.kt
- Reformatted code for better readability, specifically around the
FruittieNetworkApi
class. Lines 28-31
- Reformatted code for better readability, specifically around the
- Fruitties/shared/src/commonMain/kotlin/com/example/fruitties/viewmodel/MainViewModel.kt
- Reformatted code for better readability, specifically around the
MainViewModel
class and its properties. Lines 38-52
- Reformatted code for better readability, specifically around the
- Fruitties/shared/src/iosMain/kotlin/com/example/fruitties/di/Factory.native.kt
- Replaces
dbFileName
withDB_FILE_NAME
. Line 34 - Reformatted code for better readability, specifically around the
Factory
class and its functions. Lines 33-57
- Replaces
- Fruitties/shared/src/iosMain/kotlin/com/example/fruitties/di/viewmodel/IOSViewModelOwner.kt
- Reformatted code for better readability, specifically around the
IOSViewModelOwner
class. Lines 14-22
- Reformatted code for better readability, specifically around the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A spotless code,
A lint-free, shining grace,
Fruits of our labor.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request focuses on updating formatting, library versions, and addressing specific issues like duplicate declarations and compiler warnings. The changes seem well-organized and address the stated goals.
Summary of Findings
- Redundant code: The
kotlinx-coroutines-core-iossimulatorarm64
dependency is declared twice inlibs.versions.toml
, which is redundant and should be removed. - Inconsistent code formatting: There are some inconsistencies in code formatting, such as extra line breaks and indentation, which could be improved for better readability.
- Duplicated bundle ID definition: The bundle ID is defined as a constant in
shared/build.gradle.kts
and also hardcoded iniosApp/iosApp.xcodeproj/project.pbxproj
. It should be defined in one place to avoid inconsistencies.
Merge Readiness
The pull request is almost ready for merging. Addressing the redundant code, inconsistent formatting, and duplicated bundle ID definition would improve the overall quality and maintainability of the code. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.
kotlinx-coroutines-core-iosarm64 = { module = "org.jetbrains.kotlinx:kotlinx-kotlinx-coroutines-core-iosarm64", version.ref = "kotlinx-coroutines" } | ||
kotlinx-coroutines-core-iossimulatorarm64 = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core-iossimulatorarm64", version.ref = "kotlinx-coroutines" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val colors = | ||
if (darkTheme) { | ||
darkColorScheme( | ||
primary = Color(0xFFBB86FC), | ||
secondary = Color(0xFF03DAC5), | ||
tertiary = Color(0xFF3700B3), | ||
) | ||
} else { | ||
lightColorScheme( | ||
primary = Color(0xFF6200EE), | ||
secondary = Color(0xFF03DAC5), | ||
tertiary = Color(0xFF3700B3), | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider reducing the extra line breaks to improve readability. It's a bit dense with all the extra spacing.
val colors = if (darkTheme) {
darkColorScheme(
primary = Color(0xFFBB86FC),
secondary = Color(0xFF03DAC5),
tertiary = Color(0xFF3700B3),
)
} else {
lightColorScheme(
primary = Color(0xFF6200EE),
secondary = Color(0xFF03DAC5),
tertiary = Color(0xFF3700B3),
)
}
val typography = | ||
Typography( | ||
bodyMedium = | ||
TextStyle( | ||
fontFamily = FontFamily.Default, | ||
fontWeight = FontWeight.Normal, | ||
fontSize = 16.sp, | ||
), | ||
) | ||
} else { | ||
lightColorScheme( | ||
primary = Color(0xFF6200EE), | ||
secondary = Color(0xFF03DAC5), | ||
tertiary = Color(0xFF3700B3), | ||
val shapes = | ||
Shapes( | ||
small = RoundedCornerShape(4.dp), | ||
medium = RoundedCornerShape(4.dp), | ||
large = RoundedCornerShape(0.dp), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, consider reducing the extra line breaks to improve readability.
val typography = Typography(
bodyMedium = TextStyle(
fontFamily = FontFamily.Default,
fontWeight = FontWeight.Normal,
fontSize = 16.sp,
),
)
val shapes = Shapes(
small = RoundedCornerShape(4.dp),
medium = RoundedCornerShape(4.dp),
large = RoundedCornerShape(0.dp),
)
@@ -53,22 +51,26 @@ kotlin { | |||
// project can be found here: | |||
// https://developer.android.com/kotlin/multiplatform/migrate | |||
val xcfName = "shared" | |||
val iosBundleId = "com.example.fruitties.ios" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit bundles several updates: 1. **Apply Spotless Formatting:** * Ran `./gradlew :androidApp:spotlessApply` to enforce code style consistency. 2. **Configure ktlint via `.editorconfig`:** * Added `.editorconfig` to the Fruitties project. * Configured ktlint to ignore function naming rules for Composable functions. * Disabled the standard multiline expression wrapping rule to allow for better readability in specific assignment cases. 3. **Update Dependencies:** * Updated various dependencies in `libs.versions.toml`: * agp: 8.9.1 * androidx-activityCompose: 1.10.1 * androidx-paging: 3.3.6 * androidx-room: 2.7.0-rc03 * androidx-lifecycle: 2.9.0-alpha13 * compose: 1.7.8 * dataStore: 1.1.4 * kotlin: 2.1.10 * ksp: 2.1.10-1.0.30 * pagingComposeAndroid: 3.3.6 * skie: 0.10.1 * sqlite: 2.5.0-rc03 4. **Set iOS Framework Bundle ID:** * Explicitly set the `bundleId` for iOS frameworks to `com.example.fruitties.ios` using `binaryOptions["bundleId"]`. * This resolves the compiler warning: "Please specify the bundle ID explicitly using the -Xbinary=bundleId=<id> compiler flag."
0c519e1
to
b32d1cf
Compare
[*.{kt,kts}] | ||
# Kotlin style typically requires functions to start with a lowercase letter. | ||
# Composable functions should start with a capital letter. | ||
ktlint_function_naming_ignore_when_annotated_with = Composable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try instead format using the "android" style? If I remember right this shouldn't be an issue using that style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried
ktlint_code_style = android_studio
I do not think it solves this problem and I also think it introduces a new problem.
Ktlint android_studio
doesn't use Composable annotation
android_studio: unset
https://github.com/pinterest/ktlint/blob/master/documentation/snapshot/docs/rules/standard.md#function-naming
The documentation suggests setting this yourself:
"When using Compose, you might want to configure the function-naming rule with .editorconfig property ktlint_function_naming_ignore_when_annotated_with=Composable"
Ktlint android_studio
incorrectly removes trailing commas
When I use ktlint_code_style = android_studio
and ./gradlew spotlessApply
, trailing commas are removed from mult-line lists, functions declarations, and function calls. I do not think this is the desired behavior, so the code style of android_studio
might not be exactly what we want.
darkColorScheme(
primary = Color(0xFFBB86FC),
secondary = Color(0xFF03DAC5),
- tertiary = Color(0xFF3700B3),
+ tertiary = Color(0xFF3700B3)
)
Most android
org samples do not use android_studio
. It is declared in the PerformanceCodelab. When I run ./gradlew spotlessApply
it also removes the trailing commas:
https://github.com/android/codelab-android-compose/blob/main/PerformanceCodelab/config/.editorconfig
listOf(
FrameTimingMetric(),
- TraceSectionMetric("PhasesComposeLogo", TraceSectionMetric.Mode.Sum),
+ TraceSectionMetric("PhasesComposeLogo", TraceSectionMetric.Mode.Sum)
)
Primarily driven by
./gradlew :androidApp:spotlessApply
and./gradlew lint
./gradlew :androidApp:spotlessApply
libs.versions.toml
iossimulatorarm64
instead ofiosarm64
ktlint_function_naming_ignore_when_annotated_with = Composable