Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Confronting quality problems of long-lived codebase

Confronting quality problems of long-lived codebase

長く生きるコードベースの「品質」問題に向き合う

Munetoshi Ishikawa, LINE Corporation

LINE Developers

October 19, 2021
Tweet

More Decks by LINE Developers

Other Decks in Technology

Transcript

  1. Long-lived codebase Successful / important product - Many developers -

    Many lines of code - Long history Codebase Developers Team A Team B history “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  2. Communication app, “LINE” for Android • Started 10 years ago

    • More than 80 active developers • Many LoC - Kotlin: 1.6M LoC, 15k Files - Java: 0.81M LoC, 4.8k Files
  3. Problems of a long-lived codebase 1/2 Loose "context" of source

    code - Only author and reviewers know Team A Team B merge fetch request review Codebase “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  4. Independent teams making silos - Can be an obstacle against

    updating a rule, library, or best-practice Team A Team B Team C library rule Problems of a long-lived codebase 2/2
  5. Objectives Record for history and background Understandable code without context

    Break silos for knowledge and develop culture sharing
  6. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  7. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  8. Two phase review: Problem Inflexible relationships between authors and reviewers

    Team A Team B code review code review “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  9. Two phase review: Concept Review by someone who does not

    know the code - First reviewer: Normal review - Second reviewer: Readability review Team A Team B first review second review “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  10. Two phase review: Process 1. Conduct “normal” review 2. Assign

    reviewer randomly after review Team A Team B first review request Random assign bot notify “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  11. Two phase review: Benefits • Code readability check without “context”

    • Feedback to reviewers • Inter-team knowledge sharing
  12. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  13. May overlook architecture design issue only with code review Design

    doc: Problem LGTM LGTM LGTM + + “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  14. Design doc: Concept Discuss on design before implementation - Makes

    technical issues clear - No concrete format - Can be updated during implementation Design doc Implement
  15. Design doc: Our tips Not mandatory: write only for middle

    to long task Small step to start: don’t try to create a perfect doc before discussion Traceable: put link to pull request
  16. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  17. "Code readability" presentation: Problem Need to share knowledge of good

    code and structure - In order to make “two phase review” and “design doc” effective
  18. "Code readability" presentation 8 hour sessions for ideas to improve

    code readability Content: naming, state management, dependency… Code Readability Munetoshi Ishikawa https://speakerdeck.com/munetoshi/code-readability
  19. "Code readability" presentation: Example 1/2 “Split by object”: Two ways

    to split a function condition or object Account type
 
 UI Normal Premium Background Gray Gold Account icon Should split function for each UI, not account type “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  20. "Code readability" presentation: Example 2/2 Benefits of “Split by object”:

    readability, robustness, and further refactoring Case 1: split by condition fun updateAccountViews(accountType: AccountType) = when(accountType) { AccountType.NORMAL -> updateViewsForNormalAccount() AccountType.PREMIUM -> updateViewsForPremiumAccount() }
 Case 2: split by object fun updateAccountViews(accountType: AccountType) { updateBackgroundColor(accountType) updateIconImage(accountType) }
  21. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  22. Tech consultant: Problem Hard to standardize skills and cultures -

    Architectural design, rules, best practices, libraries, process
  23. Tech consultant: Basic idea A senior engineer joins a team

    - Reviews pull requests thoroughly - Shares feedbacks: knowledge, skill, process Team senior eng. review request feedback write documents update structure “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  24. Tech consultant: Check list Project rule: conventions, platform usage… Task

    management: discussion, design doc, scheduling… Architectural design: layers, dependencies, state… Code details: naming, logic flow, correctness…
  25. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  26. Review committee: Basic idea 1. A team voluntarily picks a

    “merged” pull-request once per week 2. A senior engineer reviews the picked pull-request 3. The senior engineer then writes and shares a weekly report Team A Team B senior eng. send a merged PR feedback write report share “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/
  27. Review committee: Report example Smell: sealed class without “a child

    specific property” sealed class AccountType(val accountId: Long, val accountName: String) { class NormalAccount(accountId: Long, accountName: String) : AccountType(accountId, accountName) class PremiumAccount(accountId: Long, accountName: String) : AccountType(accountId, accountName) } Fixed code: extract type as enum class AccountModel(val id: Long, val name: String, val type: AccountType) enum class AccountType { NORMAL, PREMIUM }
  28. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  29. Future plan One misleading point in this presentation “A senior

    engineer” in “tech consulting” and “review committee”
 == only me Need to remove single point of failure and scale out
  30. Summary Efforts improving quality for long-lived code - Problem: Code

    context and siloing of teams” - Five solutions Current culture - Improve everyone’s skill level effectively - Improve everyone’s development environment