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.

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

✨ DataStore モジュールを追加 #110

Merged
merged 5 commits into from
Dec 4, 2023
Merged

Conversation

tatsutakein
Copy link
Member

@tatsutakein tatsutakein commented Dec 4, 2023

Issue

  • close #ISSUE_NUMBER 🦕

概要

DataStore モジュールを追加します。

レビュー観点

特になし

レビューレベル

  • Lv0: まったく見ないで Approve する
  • Lv1: ぱっとみて違和感がないかチェックして Approve する
  • Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証して Approve する
  • Lv3: 実際に環境で動作確認したうえで Approve する

レビュー優先度

  • すぐに見てもらいたい ( hotfix など ) 🚀
  • 今日中に見てもらいたい 🚗
  • 今日〜明日中で見てもらいたい 🚶
  • 数日以内で見てもらいたい 🐢

参考リンク

スクリーンショット

Before After

Summary by CodeRabbit

  • 新機能

    • データストア関連の機能が追加されました。
    • JSONシリアライズの設定が追加され、データの取り扱いが改善されました。
  • バグ修正

    • Supabaseクライアントの設定が更新され、安定性が向上しました。
  • ドキュメント

    • 新しいデータストアモジュールのドキュメントが追加されました。
  • リファクタリング

    • データストアのコード構造が改善され、メンテナンスが容易になりました。
  • スタイル

    • コードのスタイルが一貫性を持つように更新されました。
  • テスト

    • 新しいデータストア機能に対するテストが追加されました。
  • チョア

    • ビルド設定が更新され、依存関係が最適化されました。
  • リバート

    • 一部の設定が以前のバージョンに戻されました。

@tatsutakein tatsutakein requested a review from a team as a code owner December 4, 2023 08:41
Copy link

coderabbitai bot commented Dec 4, 2023

Walkthrough

データストア関連の機能がプロジェクトに追加されたことが概要から読み取れます。これには、新しいモジュールの導入、依存関係の追加、シリアライゼーションの設定の更新などが含まれています。全体的に、データの取り扱いと保存のためのコードベースの拡張が行われているようです。

Changes

ファイルパス 変更概要
app/.../KmpEntryPoint.kt, app/.../NitoApp.kt dataStoreModuleのインポートと初期化を追加
app/shared/build.gradle.kts, core/data/build.gradle.kts projects.core.datastoreへの依存関係を追加
core/common/build.gradle.kts, core/datastore/build.gradle.kts 新しい依存関係とプラグインの設定を追加
core/common/.../NitoJsonSettings.kt 新しいJson設定の定数nitoJsonSettingsを追加
core/datastore/.../DataStore.kt, core/datastore/.../SettingsDataStore.kt 新しいDataStoreインターフェースとSettingsDataStoreクラスを追加
core/datastore/.../SettingsFactory.kt, core/datastore/.../di/DataStoreModule.kt Settingsの生成関数とdataStoreModuleを追加
core/network/.../SupabaseClient.kt, core/network/.../di/SupabaseClientModule.kt シリアライゼーションの設定をnitoJsonSettingsに置き換え
settings.gradle.kts :core:datastoreモジュールをプロジェクトに追加

🐰✨
データを守る新しい砦、
コードの森に静かに立つ。
バグを遠ざけ、安全を保ち、
データストア、新たな旅立ち。


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 7

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ec5df5b and a1dd63c.
Files ignored due to filter (1)
  • gradle/libs.versions.toml
Files selected for processing (14)
  • app/ios-combined/src/commonMain/kotlin/club/nito/ios/combined/KmpEntryPoint.kt (2 hunks)
  • app/shared/build.gradle.kts (1 hunks)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoApp.kt (2 hunks)
  • core/common/build.gradle.kts (1 hunks)
  • core/common/src/commonMain/kotlin/club/nito/core/common/NitoJsonSettings.kt (1 hunks)
  • core/data/build.gradle.kts (1 hunks)
  • core/datastore/build.gradle.kts (1 hunks)
  • core/datastore/src/commonMain/kotlin/club/nito/core/datastore/DataStore.kt (1 hunks)
  • core/datastore/src/commonMain/kotlin/club/nito/core/datastore/SettingsDataStore.kt (1 hunks)
  • core/datastore/src/commonMain/kotlin/club/nito/core/datastore/SettingsFactory.kt (1 hunks)
  • core/datastore/src/commonMain/kotlin/club/nito/core/datastore/di/DataStoreModule.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/SupabaseClient.kt (2 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/di/SupabaseClientModule.kt (2 hunks)
  • settings.gradle.kts (1 hunks)
Additional comments: 10
app/ios-combined/src/commonMain/kotlin/club/nito/ios/combined/KmpEntryPoint.kt (2)
  • 5-11: 新しいdataStoreModuleのインポートが追加されています。これは、DataStoreモジュールがプロジェクトに統合されていることを示しています。

  • 31-37: init関数内のmodules関数呼び出しにdataStoreModuleが追加されています。これは、新しいDataStoreモジュールがKoin DIコンテナに登録されていることを示しています。

app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoApp.kt (2)
  • 11-17: 新しいdataStoreModuleのインポートと初期化への追加は、アプリケーションのデータ処理またはストレージメカニズムの拡張または変更を示しています。これはPRの目的と一致しており、問題は見当たりません。

  • 42-48: dataStoreModuleNitoApp関数のmodulesリストに追加されています。これは、新しいデータストアモジュールがアプリケーションの依存性注入セットアップに組み込まれたことを示しており、PRの目的と一致しています。

core/data/build.gradle.kts (1)
  • 17-23: > 💡 NOTE

Codebase verification is a beta feature.

新しい依存関係 projects.core.datastore が以下のファイルに追加されています:

  • core/data/build.gradle.kts
  • app/shared/build.gradle.kts

この依存関係がプロジェクトに適切に統合されており、必要なコードベースの更新が行われていることを確認してください。

core/datastore/build.gradle.kts (1)
  • 1-30: この変更は、新しいDataStoreモジュールのbuild.gradle.ktsファイルにおけるプラグインの設定、Androidの名前空間の設定、および依存関係の追加を含んでおり、プルリクエストの目的と一致しています。特に問題は見受けられません。
core/datastore/src/commonMain/kotlin/club/nito/core/datastore/di/DataStoreModule.kt (1)
  • 11-18: コードはPRの目的と一致しており、DataStoreのインスタンスを提供するためのモジュールが正しく定義されています。ただし、ExperimentalSettingsApiを使用していることに注意してください。このAPIは実験的であり、将来的に変更される可能性があるため、プロダクションコードでの使用には注意が必要です。将来的にAPIが安定版になるまでのリスクを理解し、適切な対応を検討してください。
core/network/src/commonMain/kotlin/club/nito/core/network/di/SupabaseClientModule.kt (2)
  • 3-3: 新しいJSON設定のインポートが追加されました。これは、nitoJsonSettingsSupabaseClientModuleで使用されることを意味します。

  • 18-19: JsonのシングルトンがnitoJsonSettingsを直接使用するように更新されました。

settings.gradle.kts (1)
  • 28-34: この変更は、:core:datastore モジュールがプロジェクトの設定に追加されたことを示しています。これは、プルリクエストの目的と提供されたサマリーに一致しており、明らかな問題は見受けられません。

Comment on lines +11 to +12
@OptIn(ExperimentalSettingsApi::class)
private val suspendSettings: SuspendSettings = settings.toSuspendSettings(),
Copy link

Choose a reason for hiding this comment

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

実験的なAPI ExperimentalSettingsApi を使用しています。このAPIの使用にはリスクが伴う可能性があるため、その使用理由をコメントで説明することをお勧めします。

+    // Experimental API is used here. Ensure that the risks are understood and acceptable.
    @OptIn(ExperimentalSettingsApi::class)

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
@OptIn(ExperimentalSettingsApi::class)
private val suspendSettings: SuspendSettings = settings.toSuspendSettings(),
// Experimental API is used here. Ensure that the risks are understood and acceptable.
@OptIn(ExperimentalSettingsApi::class)
private val suspendSettings: SuspendSettings = settings.toSuspendSettings(),

Comment on lines +1 to +3
package club.nito.core.datastore

public sealed interface DataStore
Copy link

Choose a reason for hiding this comment

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

パッケージレベルのドキュメンテーションやコメントがないため、このインターフェースの目的や使い方についての説明があると、保守性や明確性が向上します。

Comment on lines +5 to +9
internal fun createSettings() = try {
Settings()
} catch (e: Exception) {
error("Failed to create default settings")
}
Copy link

Choose a reason for hiding this comment

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

エラーメッセージに例外の詳細を含めることで、デバッグ時に役立つ情報を提供することができます。例外のスタックトレースをログに記録することも検討してください。

-    error("Failed to create default settings")
+    error("Failed to create default settings", e)

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
internal fun createSettings() = try {
Settings()
} catch (e: Exception) {
error("Failed to create default settings")
}
internal fun createSettings() = try {
Settings()
} catch (e: Exception) {
error("Failed to create default settings", e)
}

Comment on lines +3 to +20
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonNamingStrategy

public val nitoJsonSettings: Json = Json {
encodeDefaults = true
isLenient = true
allowSpecialFloatingPointValues = true
allowStructuredMapKeys = true
prettyPrint = false
useArrayPolymorphism = false
ignoreUnknownKeys = true
coerceInputValues = true
useAlternativeNames = false

@OptIn(ExperimentalSerializationApi::class)
namingStrategy = JsonNamingStrategy.SnakeCase
}
Copy link

Choose a reason for hiding this comment

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

nitoJsonSettingsの設定にExperimentalSerializationApiが使用されています。将来的にAPIが変更された場合に備えて、この実験的APIの使用に関するリスクをプロジェクトチームと共有し、必要に応じてフォールバックメカニズムを検討することをお勧めします。

Comment on lines 20 to 22

defaultSerializer = KotlinXSerializer(json)
}
Copy link

Choose a reason for hiding this comment

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

Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [11-22]

セキュリティ上の懸念があります。SupabaseのURLとキーがハードコードされており、これが機密情報である場合、コード内に直接含めるべきではありません。これらの値は環境変数や設定ファイルから読み込むように変更することをお勧めします。

-    supabaseUrl = "https://gtfjukrauyhrbglrzlva.supabase.co",
-    supabaseKey = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6Imd0Zmp1a3JhdXlocmJnbHJ6bHZhIiwicm9sZSI6ImFub24iLCJpYXQiOjE2OTkwMDQ3NTgsImV4cCI6MjAxNDU4MDc1OH0.zRjlzXVyz4vBM8Tb8GcpyPyTkCmOwdV-Xs18Agw2w-E",
+    supabaseUrl = System.getenv("SUPABASE_URL"),
+    supabaseKey = System.getenv("SUPABASE_KEY"),

Comment on lines 15 to 21
dependencies {
implementation(projects.core.common)
implementation(projects.core.model)
implementation(projects.core.datastore)
implementation(projects.core.data)
implementation(projects.core.network)
implementation(projects.core.domain)
Copy link

Choose a reason for hiding this comment

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

💡 NOTE
Codebase verification is a beta feature.

projects.core.datastore モジュールが commonMain 依存関係に追加されましたが、core.datastore モジュールの build.gradle.kts ファイルに関する情報が見つかりませんでした。バージョン競合や追加の設定が必要かどうかを確認するためには、このファイルの内容を確認する必要があります。プロジェクトの設定ファイルを全体的に見直して、この変更が他のモジュールとの競合を引き起こさないことを確認してください。

Comment on lines 16 to +19
dependencies {
implementation(libs.kotlinxCoroutinesCore)
implementation(libs.kotlinxDatetime)
implementation(libs.kotlinSerializationJson)
Copy link

Choose a reason for hiding this comment

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

💡 NOTE
Codebase verification is a beta feature.

kotlinSerializationJsonライブラリがプロジェクトの依存関係に追加されたことを確認しました。既存のコードでJSONシリアライズに関連する複数の箇所が使用されていることが確認されています。この新しい依存関係が既存のシリアライズ設定や実装に影響を与えないか、詳細なレビューが必要です。

@tatsutakein tatsutakein merged commit 1801fe3 into main Dec 4, 2023
4 checks passed
@tatsutakein tatsutakein deleted the rt/add-datastore-module branch December 4, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant