Go to Qiita Advent Calendar Top
LoginSignup
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@majoLibrary

【Android】レビューしやすいPRの設計

Last updated at Posted at 2025-09-05

はじめに

大きくまとまったPRは、レビューが進みにくく、指摘が錯綜して結論が出るまでに時間がかかりがちです。そうしているうちにベースブランチとのコンフリクトが増え、手戻りや修正が積み重なり、リリース全体が遅れてしまうことも少なくありません。

一方で、目的ごとに小さく整理されたPRは「この変更では何を見るか」が明確で、レビュアも短時間で集中して確認できます。結果として合意形成が早まり、後工程の不確実性も減ります。

この記事では、Android開発の現場でPRを小さく切るための基本として「基盤を先に切る」「責務を混ぜない」という2つの観点を取り上げます。

1. 基盤を先に切る

原則

  • 完成形を出さず、小粒なベースだけをPRにする
    • 例：UseCaseの契約1つ、UiStateの定義1つ、Activityの殻だけなど
  • 目的は“合意形成”であり、動作実装ではない
    • 命名・戻り値・状態の網羅といった取り決めを先に固める
  • 実装・UI・結線は後続PRに回す
    • 役割を分けて積み上げやすくするため、基盤PRは呼び水として小さく出す

効果

  • レビュー観点が一意になる
    • 命名・戻り値・状態の網羅性・公開APIの形だけを短時間で確認できる
  • 合意形成が早まる
    • 実装に入る前の段階で取り決めを固めるため、後工程の手戻りが減る
  • リスク分離ができる
    • 土台／実装／UIを独立にロールバックでき、影響範囲を最小化できる
  • 並行作業に強くなる
    • 基盤を早期に共有することで、方向性のズレや差し戻しを減らし、小さな後続PRを切りやすくする
基盤PR サンプルコード

1) UseCase の契約（1ファイル）

// 呼び出し側が合意すべき入口だけ定義
interface GetUserUseCase {
    suspend operator fun invoke(id: String): Result<User>
}

2) 画面の語彙（UiState / UiEvent）（1ファイル）

// 画面が扱う最小の語彙だけ
sealed interface UserUiState {
    data object Loading : UserUiState
    data class Content(val user: User) : UserUiState
    data class Error(val message: String) : UserUiState
}

sealed interface UserEvent {
    data object Retry : UserEvent
}

3) ドメイン最小モデル（1ファイル）

// まずは必要最小限の構造だけ
data class User(val id: String, val name: String)

4) ViewModelの公開APIだけ（中身は後続）（1ファイル）

// 公開面の“約束”だけを固める（実装は後続PR）
class UserViewModel(
    private val getUser: GetUserUseCase
) : ViewModel() {
    private val _uiState = MutableStateFlow<UserUiState>(UserUiState.Loading)
    val uiState: StateFlow<UserUiState> = _uiState

    fun load(id: String) {}
    fun onEvent(event: UserEvent) {}
}

5) Activityの殻（結線の最小形）（1ファイル）

class UserActivity : ComponentActivity() {
    // VM の入手方法は後続PRで（HiltでもFactoryでもOK）
    private val viewModel: UserViewModel by lazy { TODO("Provide later") }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent { /* 後続PRで Composable を追加。今は殻だけ */ }
    }
}

ここまでで「この画面はGetUserUseCaseを呼び、UserUiStateを流す」という土台の合意ができます。実装は一切入れません。

2. 責務を混ぜない

原則

基盤が整ったら、次は実際に処理やUIを積んでいきます。
そのときに大事なのが「責務を混ぜない」こと。

UI（Compose）とロジック（ViewModel/UseCase）、あるいはリファクタリングと新機能追加のように、目的が異なる変更は同じPRに含めないようにします。

効果

  • レビュー観点が明確になる
    • UIだけなら見た目・レイアウトに集中できる
    • ロジックだけなら状態遷移や非同期処理に集中できる
  • 指摘が整理されやすい
    • 変更の意図が一貫しているため、不要な指摘や見落としが減る
  • リスクを分離できる
    • UIだけ、リファクタだけ、といった単位で安全にリリース判断やロールバックが可能になる

代表例

  1. UI専用PR：Composableやレイアウトの改善だけを含める
  2. ロジック専用PR：状態管理や非同期処理など、ビジネスロジックの変更に限定する
サンプルコード

UI専用PR（見た目と結線だけ追加）

class UserActivity : ComponentActivity() {
    private val viewModel: UserViewModel by lazy { TODO("Provide later") }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent { UserScreen(viewModel) }
    }
}

@Composable
fun UserScreen(viewModel: UserViewModel) {
    val uiState by viewModel.uiState.collectAsState()

    when (uiState) {
        is UserUiState.Loading -> Text("Loading…")
        is UserUiState.Content -> {
            val content = uiState as UserUiState.Content
            Column {
                Text("Hello ${content.user.name}")
                Button(onClick = { viewModel.refresh(content.user.id) }) {
                    Text("Reload")
                }
            }
        }
        is UserUiState.Error -> {
            val error = uiState as UserUiState.Error
            Column {
                Text("Error: ${error.message}")
                Button(onClick = { viewModel.onEvent(UserEvent.Retry) }) {
                    Text("Retry")
                }
            }
        }
    }
}

ここではUIの見た目・文言・イベント結線だけレビュー対象。ViewModelの実装は空のまま。

ロジック専用PR (状態遷移・非同期処理のみ)

class UserViewModel(
    private val getUser: GetUserUseCase,
) : ViewModel() {
    private val _uiState = MutableStateFlow<UserUiState>(UserUiState.Loading)
    val uiState: StateFlow<UserUiState> = _uiState

    private var lastId: String? = null

    fun refresh(id: String) {
        lastId = id
        viewModelScope.launch {
            _uiState.value = UserUiState.Loading
            val result = getUser(id)
            _uiState.value = result.fold(
                onSuccess = { user -> UserUiState.Content(user) },
                onFailure = { throwable -> UserUiState.Error(throwable.message ?: "読み込みに失敗しました") }
            )
        }
    }

    fun onEvent(event: UserEvent) {
        when (event) {
            is UserEvent.Retry -> lastId?.let { refresh(it) }
        }
    }
}

ここでは ViewModelの状態遷移と非同期処理の正しさだけレビュー対象。
UIは一切触らないので、UI専用PRと並行してレビューができる。

レビューしやすいPRの基準

レビューのしやすさは「量」だけでなく「意味のまとまり」によって決まります。
以下の基準を基本の目安とすると、PRはぐっと読みやすくなります。

  • コード差分が 500行以下
    • 理想としては 50 行程度、200〜400行以下が推奨されるとの調査結果もあり、レビューやマージ速度・リビジョンの質が向上します
      https://www.gustavwengel.dk/pr-sizes
  • 変更ファイル数（リソースを除く）が 5つ以下
  • 基盤を先に切っている
    • Repository の interface、UiState・UiEvent、最小のモデル定義など、後続の実装やUIに積みやすい“小粒のベース”だけを独立PRにする
  • 責務を混ぜていない
    • UIとロジック、リファクタリングと機能追加など異なる責務は分ける

関連記事

基盤を先に切る

→ PRを積み上げるとき、まず基盤となるPRを出してから後続を重ねる「stacked PR」の考え方。この記事でいう「基盤を先に切る」に相当します。

責務を混ぜない

→ 「小さく」「焦点を絞る」PRを推奨。複数の責務を1PRに混ぜるとレビュー効率が落ちることを具体的に指摘しています。この記事でいう「責務を混ぜない」に直結します。

→ 小さなPRが品質・レビュー速度・理解のしやすさに寄与する理由を解説。責務を分けて小さくすることの価値を裏付けます。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?