Save 37% off PRO during our Black Friday Sale! »

Confronting quality problems of long-lived codebase

Confronting quality problems of long-lived codebase

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

Munetoshi Ishikawa, LINE Corporation

53850955f15249a1a9dc49df6113e400?s=128

LINE Developers
PRO

October 19, 2021
Tweet

Transcript

  1. Munetoshi Ishikawa, LINE Corporation Confronting quality problems of long-lived codebase

    ௕͘ੜ͖Δίʔυϕʔεͷʮ඼࣭ʯ໰୊ʹ޲͖߹͏
  2. 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/
  3. 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
  4. 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/
  5. 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
  6. Objectives Record for history and background Understandable code without context

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

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

    “Code readability” presentation 4. Tech consultant 5. Review committee
  9. 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/
  10. 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/
  11. 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/
  12. Two phase review: Benefits • Code readability check without “context”

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

    “Code readability” presentation 4. Tech consultant 5. Review committee
  14. 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/
  15. Design doc: Concept Discuss on design before implementation - Makes

    technical issues clear - No concrete format - Can be updated during implementation Design doc Implement
  16. Design doc: Contents Objective Background Abstract design Design details Concerns,

    known issues Our design doc example for refactoring
  17. 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
  18. Our effort 1. Two phase review 2. Design doc 3.

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

    code and structure - In order to make “two phase review” and “design doc” effective
  20. "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
  21. "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/
  22. "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) }
  23. "Code readability" presentation: Achievement Available as - External resource: https://speakerdeck.com/munetoshi/code-readability

    - In-company trainings: LINE Class, Z Academia
  24. Our effort 1. Two phase review 2. Design doc 3.

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

    Architectural design, rules, best practices, libraries, process
  26. 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/
  27. 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…
  28. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  29. Review committee: Problem Requires knowledge sharing with all teams -

    Tech consulting is for only one team
  30. 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/
  31. 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 }
  32. Our effort 1. Two phase review 2. Design doc 3.

    “Code readability” presentation 4. Tech consultant 5. Review committee
  33. 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
  34. 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