Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove desugaring from this project #1485

Closed
wants to merge 3 commits into from
Closed

Remove desugaring from this project #1485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd9b4b9
Remove desugaring.
Jaehwa-Noh May 31, 2024
1eef3d0
Remove desugaring.
Jaehwa-Noh May 31, 2024
1eca0a0
Fix spotless.
Jaehwa-Noh May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/KotlinAndroid.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import org.gradle.api.Project
import org.gradle.api.plugins.JavaPluginExtension
import org.gradle.kotlin.dsl.assign
import org.gradle.kotlin.dsl.configure
import org.gradle.kotlin.dsl.dependencies
import org.gradle.kotlin.dsl.provideDelegate
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
import org.jetbrains.kotlin.gradle.dsl.KotlinAndroidProjectExtension
Expand All @@ -47,15 +46,10 @@ internal fun Project.configureKotlinAndroid(
// https://developer.android.com/studio/write/java11-minimal-support-table
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
isCoreLibraryDesugaringEnabled = true
}
}

configureKotlin<KotlinAndroidProjectExtension>()

dependencies {
add("coreLibraryDesugaring", libs.findLibrary("android.desugarJdkLibs").get())
}
}

/**
Expand Down
13 changes: 4 additions & 9 deletions core/ui/src/main/kotlin/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic
import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
import com.google.samples.apps.nowinandroid.core.model.data.UserNewsResource
import kotlinx.datetime.Instant
import kotlinx.datetime.toJavaInstant
import kotlinx.datetime.toJavaZoneId
import java.time.format.DateTimeFormatter
import java.time.format.FormatStyle
import java.text.DateFormat
import java.util.Locale

/**
Expand Down Expand Up @@ -240,11 +237,9 @@ fun NotificationDot(
}

@Composable
fun dateFormatted(publishDate: Instant): String = DateTimeFormatter
.ofLocalizedDate(FormatStyle.MEDIUM)
.withLocale(Locale.getDefault())
.withZone(LocalTimeZone.current.toJavaZoneId())
.format(publishDate.toJavaInstant())
fun dateFormatted(publishDate: Instant): String = DateFormat
.getDateInstance(DateFormat.MEDIUM, Locale.getDefault())
.format(publishDate.toEpochMilliseconds())
Copy link

Choose a reason for hiding this comment

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

DateFormat is not thread-safe or immutable and DateTimeFormatter is.

ideally, the formatter would be remember'd (and thread-safe and immutable)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you to review it.
Let me says my opinion.

I checked the java documentation, it said

Synchronization

Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.

I don't think my DateFormat is called on multiple thread, it's only called on main thread.
And It has been created new DateFormat instance as NewsResourceCardExpanded is composed.


@Composable
fun NewsResourceMetaData(
Expand Down
2 changes: 0 additions & 2 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[versions]
accompanist = "0.34.0"
androidDesugarJdkLibs = "2.0.4"
# AGP and tools should be updated together
androidGradlePlugin = "8.4.0"
androidTools = "31.4.0"
Expand Down Expand Up @@ -63,7 +62,6 @@ turbine = "1.1.0"

[libraries]
accompanist-permissions = { group = "com.google.accompanist", name = "accompanist-permissions", version.ref = "accompanist" }
android-desugarJdkLibs = { group = "com.android.tools", name = "desugar_jdk_libs", version.ref = "androidDesugarJdkLibs" }
androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "androidxActivity" }
androidx-appcompat = { group = "androidx.appcompat", name = "appcompat", version.ref = "androidxAppCompat" }
androidx-benchmark-macro = { group = "androidx.benchmark", name = "benchmark-macro-junit4", version.ref = "androidxMacroBenchmark" }
Expand Down
Loading