Slide 1

Slide 1 text

Munetoshi Ishikawa, LINE Corporation Confronting quality problems of long-lived codebase ௕͘ੜ͖Δίʔυϕʔεͷʮ඼࣭ʯ໰୊ʹ޲͖߹͏

Slide 2

Slide 2 text

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/

Slide 3

Slide 3 text

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

Slide 4

Slide 4 text

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/

Slide 5

Slide 5 text

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

Slide 6

Slide 6 text

Objectives Record for history and background Understandable code without context Break silos for knowledge and develop culture sharing

Slide 7

Slide 7 text

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

Slide 8

Slide 8 text

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

Slide 9

Slide 9 text

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/

Slide 10

Slide 10 text

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/

Slide 11

Slide 11 text

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/

Slide 12

Slide 12 text

Two phase review: Benefits • Code readability check without “context” • Feedback to reviewers • Inter-team knowledge sharing

Slide 13

Slide 13 text

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

Slide 14

Slide 14 text

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/

Slide 15

Slide 15 text

Design doc: Concept Discuss on design before implementation - Makes technical issues clear - No concrete format - Can be updated during implementation Design doc Implement

Slide 16

Slide 16 text

Design doc: Contents Objective Background Abstract design Design details Concerns, known issues Our design doc example for refactoring

Slide 17

Slide 17 text

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

Slide 18

Slide 18 text

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

Slide 19

Slide 19 text

"Code readability" presentation: Problem Need to share knowledge of good code and structure - In order to make “two phase review” and “design doc” effective

Slide 20

Slide 20 text

"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

Slide 21

Slide 21 text

"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/

Slide 22

Slide 22 text

"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) }

Slide 23

Slide 23 text

"Code readability" presentation: Achievement Available as - External resource: https://speakerdeck.com/munetoshi/code-readability - In-company trainings: LINE Class, Z Academia

Slide 24

Slide 24 text

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

Slide 25

Slide 25 text

Tech consultant: Problem Hard to standardize skills and cultures - Architectural design, rules, best practices, libraries, process

Slide 26

Slide 26 text

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/

Slide 27

Slide 27 text

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…

Slide 28

Slide 28 text

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

Slide 29

Slide 29 text

Review committee: Problem Requires knowledge sharing with all teams - Tech consulting is for only one team

Slide 30

Slide 30 text

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/

Slide 31

Slide 31 text

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 }

Slide 32

Slide 32 text

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

Slide 33

Slide 33 text

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

Slide 34

Slide 34 text

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