Skip to content

Conversation

@miosakuma
Copy link
Contributor

hello アプリを回転時に Activity の再生成が行われ Sora から切断させることを防ぐ設定を AndroidManifest.xml に入れました。
回転を含む画面サイズの変更に対して Activity の再生成が行われない設定をしています。

参考:AndroidManifest.xml 仕様
https://developer.android.com/guide/topics/manifest/activity-element?hl=ja#config

@miosakuma
Copy link
Contributor Author

!sonnet レビューをお願い

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Claude finished @miosakuma's task —— View job


レビュー結果

【重要度: 低】指摘事項

  1. screenLayout は必要か確認すること

    • android:configChangesorientation|screenSize|smallestScreenSize|screenLayout を指定しているが、screenLayout の変更も Activity の再生成をトリガーする
    • 通常、画面回転対応であれば orientation|screenSize|smallestScreenSize で十分
    • screenLayout は画面の配置(横長/縦長の判定など)が変わったときに使用されるが、hello アプリで本当に必要か要検討
    • 参考: test/android/app/src/main/AndroidManifest.xml:20
  2. CHANGES.md の記述が正確

    • CHANGES.md に「回転時やスクリーンサイズの変更時に Activity の再起動が行われないようにする」と記載されているのは正確
    • 実装内容とリリースノートが整合している

【問題なし】確認事項

  1. 設定値は Android 公式ドキュメントに準拠

    • orientation, screenSize, smallestScreenSize は画面回転時の Activity 再生成を防ぐための標準的な設定
    • PR の説明にある公式ドキュメントへのリンクも適切
  2. 影響範囲は適切

    • test/android の hello アプリのみへの変更で、他のコードへの影響なし
    • 最小限の変更で目的を達成している
  3. コミットメッセージは適切

    • 日本語で書かれており、命令形(「〜する」形)で統一されている

総評

実装は概ね問題ないが、screenLayout が本当に必要かどうかを確認することを推奨する。不要であれば削除して orientation|screenSize|smallestScreenSize のみにすることで、より意図が明確になる。


Job Run | Branch

@miosakuma
Copy link
Contributor Author

screenLayout は Android のドキュメントの以下の記載を元に設定しています。

https://developer.android.com/guide/topics/manifest/activity-element?hl=ja#config

注: アプリが Android 3.2(API レベル 13)以上を対象としている場合は、"screenLayout" と "screenSize" の構成も宣言します。これは、デバイスの画面を縦向きや横向きに切り替えると、画面レイアウトと画面サイズも変わることがあるためです。

@miosakuma miosakuma requested a review from torikizi October 9, 2025 04:17
@miosakuma miosakuma merged commit 5a3c191 into develop Nov 4, 2025
17 checks passed
@miosakuma miosakuma deleted the feature/fix-android-hello-not-to-dispose-when-orientation-change branch November 4, 2025 05:39
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.

3 participants