[DevDojo] Ship Code Faster - 2024

Slide 1

Slide 1 text

1 Conﬁdential 🚢 Ship code faster DevDojo 2024 April 2024.04.10 @kaustubh

Slide 2

Slide 2 text

2 Conﬁdential 2014-2019  - Bachelors + Masters in Computer Science, IIT Kharagpur (India)    2019 - Joined Mercari as a newgrad    Jan 2020 - Sep 2021   - US@Tokyo, Search Backend    Oct 2021 - present  - MK Product Data/Metadata, Backend  @kaustubh 

Slide 3

Slide 3 text

3 Conﬁdential Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae

Slide 4

Slide 4 text

4 Conﬁdential Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae

Slide 5

Slide 5 text

5 Conﬁdential Source: pragmaticengineer.com Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae

Slide 6

Slide 6 text

6 Conﬁdential Source: pragmaticengineer.com Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae

Slide 7

Slide 7 text

7 Conﬁdential Repeated metrics Deploys/service Ease of delivery Change failure rate No. of PRs, reviews PR Cycle Time Ease of recovery

Slide 8

Slide 8 text

8 Conﬁdential 🔁 This talk will be given in reverse

Slide 9

Slide 9 text

9 Conﬁdential Pull requests Does everyone know about them ?

Slide 10

Slide 10 text

10 Conﬁdential Code reviews Who here has done them in the past ?

Slide 11

Slide 11 text

11 Conﬁdential Code reviews Who here has done them in the past ? Q. Why don’t we let people merge their code straight to master ?

Slide 12

Slide 12 text

12 Conﬁdential Code reviews Who here has done them in the past ? Q. Why don’t we let people merge their code straight to master ? - Human errors - Optimizations - Domain education - Feature conﬂicts

Slide 13

Slide 13 text

13 Conﬁdential Code review (a short story) Developer A B

Slide 14

Slide 14 text

14 Conﬁdential Code review (a short story) Developer A B https://github.com/octocat/Hello-World/pull/1 Ref: http://slack.com/

Slide 15

Slide 15 text

15 Conﬁdential Code review (a short story) Developer A B

Slide 16

Slide 16 text

16 Conﬁdential Code review (a short story) Developer A B Comment

Slide 17

Slide 17 text

17 Conﬁdential Code review (a short story) Developer A B Comment A B

Slide 18

Slide 18 text

18 Conﬁdential Code review (a short story) Developer A B Comment A B Ref: http://slack.com/

Slide 19

Slide 19 text

19 Conﬁdential Code review (a short story) Developer A B Comment A B

Slide 20

Slide 20 text

20 Conﬁdential Code review (a short story) Developer A B Comment A B

Slide 21

Slide 21 text

21 Conﬁdential Code review (a short story) Developer A B Comment A B

Slide 22

Slide 22 text

22 Conﬁdential Code review (a short story) Developer A B Comment A B Context switch is costly ! For humans and computers Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae

Slide 23

Slide 23 text

23 Conﬁdential Code review (real story) - Previous team: Average time to review (not ship): 10-14 days - Introduce systems that aided faster review and releases - Open PRs should not go stale - Result: 3-5 days

Slide 24

Slide 24 text

24 Conﬁdential Code review (real story) - Previous team: Average time to review (not ship): 10-14 days - Introduce systems that aided faster review and releases - Open PRs should not go stale - Result: 3-5 days You can also try to introduce these practices to your teams to make the team’s release process more efﬁcient.

Slide 25

Slide 25 text

25 Conﬁdential The presenter The audience

Slide 26

Slide 26 text

26 Conﬁdential What does a stale PR mean ? How does it go stale ?

Slide 27

Slide 27 text

27 Conﬁdential What does a stale PR mean ? How does it go stale ? - PRs merged - Feature conﬂicts - Test updates - No activity

Slide 28

Slide 28 text

28 Conﬁdential Rather obvious checks Improvements Technical details Process improvements 02 03 01

Slide 29

Slide 29 text

29 Conﬁdential General note You should absolutely be direct and demanding in your code reviews.

Slide 30

Slide 30 text

30 Conﬁdential Rather obvious checks

Slide 31

Slide 31 text

31 Conﬁdential C1. Ensure your PR passes CI before request review Ref: https://github.com/

Slide 32

Slide 32 text

32 Conﬁdential C2. Summarize your code changes in description Ref: https://github.com/

Slide 33

Slide 33 text

33 Conﬁdential C2. Summarize your code changes in description If a release goes wrong, another engineer may have to revert your PR. Ref: https://github.com/

Slide 34

Slide 34 text

34 Conﬁdential C3. Give context when requesting review Ref: http://slack.com/

Slide 35

Slide 35 text

35 Conﬁdential Technical details

Slide 36

Slide 36 text

36 Conﬁdential DRY - What’s DRY ? D1. Align with Design principles

Slide 37

Slide 37 text

37 Conﬁdential D1. Align with Design principles - DRY - SOLID

Slide 38

Slide 38 text

38 Conﬁdential D1. Align with Design principles - DRY - SOLID - AHA - Discover abstractions, don’t design with them.

Slide 39

Slide 39 text

39 Conﬁdential D1. Align with Design principles - DRY - SOLID - AHA - Discover abstractions, don’t design with them. - Effective Go / language speciﬁc guidelines

Slide 40

Slide 40 text

40 Conﬁdential D1. Align with Design principles - DRY - SOLID - AHA - Discover abstractions, don’t design with them. - Effective Go / language speciﬁc guidelines NOTE: Guidelines, not rules to live by

Slide 41

Slide 41 text

41 Conﬁdential D2. getDBRecord(id string) record { // calls DB // return record if found } // HTTP endpoint, no DB call GetRecordPropertyByID( id string) { // some checks record := getDBRecord(id) return record.property; }

Slide 42

Slide 42 text

42 Conﬁdential D2. getDBRecord(id string) record { // calls DB // return record if found } // HTTP endpoint, no DB call GetRecordPropertyByID( id string) { // some checks record := getDBRecord(id) return record.property; } What is the problem here ?

Slide 43

Slide 43 text

43 Conﬁdential D2. Null case - expect nothing getDBRecord(id string) record { // calls DB // return record if found } // HTTP endpoint, no DB call GetRecordPropertyByID( id string) { // some checks record := getDBRecord(id) return record.property; } What if db returns nil ?

Slide 44

Slide 44 text

44 Conﬁdential What does that even mean DM @kaustubh D3. N+1 query

Slide 45

Slide 45 text

45 Conﬁdential D3. getDBRecord(id string) record { // calls DB } // HTTP endpoint, no DB call GetRecordByID(id string) { // some checks getDBRecord(id) // check, return data } getDBRecord(id string) record { // calls DB } // HTTP endpoint, no DB call GetRecordByID(id string) { // some checks record := getDBRecord(id) // check, return data }

Slide 46

Slide 46 text

46 Conﬁdential D3. getDBRecord(id string) record { // calls DB } // HTTP endpoint, no DB call GetRecordByID(id string) { // some checks record := getDBRecord(id) // check, return data } // HTTP endpoint, no DB call GetRecordsByIDs(ids string) { // some checks For id := range ids { records = append(records, getDBRecord(id) ) } // check, return data

Slide 47

Slide 47 text

47 Conﬁdential D3. getDBRecord(id string) record { // calls DB } // HTTP endpoint, no DB call GetRecordByID(id string) { // some checks record := getDBRecord(id) // check, return data } // HTTP endpoint, no DB call GetRecordsByIDs(ids string) { // some checks For id := range ids { records = append(records, getDBRecord(id) ) } // check, return data Is there a problem here ?

Slide 48

Slide 48 text

48 Conﬁdential D3. N+1 query getDBRecord(id string) record { // calls DB } // HTTP endpoint, no DB call GetRecordByID(id string) { // some checks record := getDBRecord(id) // check, return data } // HTTP endpoint, no DB call GetRecordsByIDs(ids string) { // some checks For id := range ids { records = append(records, getDBRecord(id) ) } // check, return data 100 DB calls for 100 ids ! Having a batch method has no beneﬁts over single method

Slide 49

Slide 49 text

49 Conﬁdential D3. N+1 query getDBRecord(id string) record { // calls DB } // HTTP endpoint, no DB call GetRecordByID(id string) { // some checks record := getDBRecord(id) // check, return data } // HTTP endpoint, no DB call GetRecordsByIDs(ids string) { // some checks For id := range ids { records = append(records, getDBRecord(id) ) } // check, return data 100 DB calls for 100 ids ! Having a batch method has no beneﬁts over single method

Slide 50

Slide 50 text

50 Confidential D4. Internal Panics - Missing file - Security flaw Recover from panics, hard to detect // pkg internal implementation // we may not be able to change it func divide(a, b int) int { if b == 0 { panic("Divide by zero") } return a / b }

Slide 51

Slide 51 text

51 Confidential D4. Internal Panics - Missing file - Security flaw Recover from panics, hard to detect - Incident // pkg internal implementation // we may not be able to change it func divide(a, b int) int { if b == 0 { panic("Divide by zero") } return a / b }

Slide 52

Slide 52 text

52 Confidential D4. Internal Panics - Missing file - Security flaw Recover from panics, hard to detect - Incident - Stripe Interview Q // pkg internal implementation // we may not be able to change it func divide(a, b int) int { if b == 0 { panic("Divide by zero") } return a / b }

Slide 53

Slide 53 text

53 Conﬁdential D5. Eager exits GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting return something; } else return null; }

Slide 54

Slide 54 text

54 Conﬁdential D5. Eager exits GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting return something; } else return null; } GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting // conversion functions // empty checks // function on record.property return something; } else return null;

Slide 55

Slide 55 text

55 Conﬁdential D5. Eager exits GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting return something; } else return null; } GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting // conversion functions // empty checks // function on record.property return something; } else return null;

Slide 56

Slide 56 text

56 Conﬁdential D5. Eager exits (opinion) GetRecordByID(id string) { // checks record := getDBRecord(id) if record == null { return nil; } else // formatting return something; } GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting // conversion functions // empty checks // function on record.property return something; } else return null;

Slide 57

Slide 57 text

57 Conﬁdential Process improvements

Slide 58

Slide 58 text

58 Conﬁdential P1. Unit tests / Integration tests What does a unit test mean? Integration test ? Why are they really required ?

Slide 59

Slide 59 text

59 Conﬁdential P1. Unit tests / Integration tests What does a unit test mean? Integration test ? Why are they really required ? Unit test - test only new functionality Integration test - Test the new function with other components Find potential issues in overall ﬂow, but not this feature alone

Slide 60

Slide 60 text

60 Conﬁdential P1. Unit tests / Integration tests HTTP server Input Output Feature 1 Feature 2 Feature N

Slide 61

Slide 61 text

61 Conﬁdential P1. Unit tests / Integration tests HTTP server Input Output Feature 1 Feature 2 Feature N Unit test Integration test

Slide 62

Slide 62 text

62 Conﬁdential P2. Test your own changes ! ● Install Dev App ● Test PR on dev app

Slide 63

Slide 63 text

63 Confidential P3. Prove you are right - Stackoverflow links - Go Playground - Official pkg docs Ref: https://github.com/

Slide 64

Slide 64 text

64 Confidential P3. Prove you are right - Stackoverflow links - Go Playground - Official pkg docs Ref: https://github.com/

Slide 65

Slide 65 text

65 Conﬁdential P4. Review your own PR in the third person Check areas of your PR that a reviewer may have concerns about - Add comments on your PR to explain your decisions. - Highlight areas for review - Performance concerns

Slide 66

Slide 66 text

66 Conﬁdential P5. Boring is good - Don’t introduce something brand new not discussed before - Are there better/simpler ways to do it ? - Your idea may not work (DB, language versions) - (Complex projects) Discuss with Tech Lead beforehand, conﬁrm your approach makes sense. - OK to explore as you go, but try not to delay release too much

Slide 67

Slide 67 text

67 Conﬁdential P5. Boring is good - Don’t introduce something brand new not discussed before - Are there better/simpler ways to do it ? - Your idea may not work (DB, language versions) - (Complex projects) Discuss with Tech Lead beforehand, conﬁrm your approach makes sense. - OK to explore as you go, but try not to delay release too much People only like surprises when it’s a gift, 🎁 not for code review 🤮

Slide 68

Slide 68 text

68 Conﬁdential P6. Commit etiquette - Each commit should do exactly one task. - There is a sweet spot for a commit size.

Slide 69

Slide 69 text

69 Conﬁdential BAD - Implement feature - address feedback GOOD P6. Commit etiquette GOOD - [chore] / [add] / [ﬁx] Depends on repo enforcements - “[CGT-123] add test for method” Shell Automations Debugging larger repos

Slide 70

Slide 70 text

70 Conﬁdential P7. Code can be neat

Slide 71

Slide 71 text

71 Conﬁdential P7. Code can be neat Ref: https://github.com/

Slide 72

Slide 72 text

72 Conﬁdential P7. Code can be neat How to do it ? Ref: https://github.com/

Slide 73

Slide 73 text

73 Conﬁdential P7. Code can be neat How to do it ? Text Details Ref: https://github.com/

Slide 74

Slide 74 text

74 Conﬁdential P7. Code can be neat How to do it ? Text Details There are tons of markdown features like this, please use them ! Ref: https://github.com/

Slide 75

Slide 75 text

75 Conﬁdential P8. Code review has a summary (save author’s time) Ref: https://github.com/

Slide 76

Slide 76 text

76 Conﬁdential P9. Know the entire codebase - Impractical at start, but expect to know in 6 months - First 3 months: WHAT features, HOW to implement - Next 3: WHO calls the service, WHY certain design decisions made, WHEN a consumer started using the service - Can reuse existing functions / abstract if required - Know how existing problems are solved

Slide 77

Slide 77 text

77 Conﬁdential Growing from a programmer to an engineer

Slide 78

Slide 78 text

78 Conﬁdential How an engineer is different from a developer - Communicate effectively with other roles (PMs, designers) - Understand the feature; think of the users, simplify architecture - Identify potential issues in implementation; yours and others - Be responsible for your domain

Slide 79

Slide 79 text

79 Conﬁdential Improving as an engineer - Read tech articles - See how others do code review / Prepare their PR - After the ﬁrst 2-3 months, see EVERY PR reviewed and merged recently - Good for feature education, decision making process

Slide 80

Slide 80 text

80 Conﬁdential Trafﬁc CPU, mem DB Service cost User Inquiry Actual implementation Releasing a new backend feature

Slide 81

Slide 81 text

81 Conﬁdential 🚢 Devdojo: Ship code faster @kaustubh 2024.04.10 Questions