[DevDojo] Ship Code Faster - 2024

This session will cover the productivity metrics used by various tech companies, discuss development and engineering practices to reduce the time from development start to feature release. It will also provide tangible steps on career progression for engineers who are new to their careers.

Transcript

1. 1 Confidential 🚢 Ship code faster DevDojo 2024 April 2024.04.10 @kaustubh
2. 2 Confidential 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

3. 3 Confidential Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae
4. 4 Confidential Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae
5. 5 Confidential Source: pragmaticengineer.com Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae
6. 6 Confidential Source: pragmaticengineer.com Ref: https://newsletter.pragmaticengineer.com/p/measuring-developer-productivity-bae
7. 7 Confidential Repeated metrics Deploys/service Ease of delivery Change failure rate No. of PRs, reviews PR Cycle Time Ease of recovery
8. 8 Confidential 🔁 This talk will be given in reverse
9. 9 Confidential Pull requests Does everyone know about them ?
10. 10 Confidential Code reviews Who here has done them in the past ?
11. 11 Confidential Code reviews Who here has done them in the past ? Q. Why don’t we let people merge their code straight to master ?
12. 12 Confidential 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 conflicts
13. 13 Confidential Code review (a short story) Developer A B
14. 14 Confidential Code review (a short story) Developer A B https://github.com/octocat/Hello-World/pull/1 Ref: http://slack.com/
15. 15 Confidential Code review (a short story) Developer A B
16. 16 Confidential Code review (a short story) Developer A B Comment
17. 17 Confidential Code review (a short story) Developer A B Comment A B
18. 18 Confidential Code review (a short story) Developer A B Comment A B Ref: http://slack.com/
19. 19 Confidential Code review (a short story) Developer A B Comment A B
20. 20 Confidential Code review (a short story) Developer A B Comment A B
21. 21 Confidential Code review (a short story) Developer A B Comment A B
22. 22 Confidential 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
23. 23 Confidential 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
24. 24 Confidential 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 efficient.
25. 25 Confidential The presenter The audience
26. 26 Confidential What does a stale PR mean ? How does it go stale ?
27. 27 Confidential What does a stale PR mean ? How does it go stale ? - PRs merged - Feature conflicts - Test updates - No activity
28. 28 Confidential Rather obvious checks Improvements Technical details Process improvements 02 03 01
29. 29 Confidential General note You should absolutely be direct and demanding in your code reviews.
30. 30 Confidential Rather obvious checks
31. 31 Confidential C1. Ensure your PR passes CI before request review Ref: https://github.com/
32. 32 Confidential C2. Summarize your code changes in description Ref: https://github.com/
33. 33 Confidential C2. Summarize your code changes in description If a release goes wrong, another engineer may have to revert your PR. Ref: https://github.com/
34. 34 Confidential C3. Give context when requesting review Ref: http://slack.com/
35. 35 Confidential Technical details
36. 36 Confidential DRY - What’s DRY ? D1. Align with Design principles
37. 37 Confidential D1. Align with Design principles - DRY - SOLID
38. 38 Confidential D1. Align with Design principles - DRY - SOLID - AHA - Discover abstractions, don’t design with them.
39. 39 Confidential D1. Align with Design principles - DRY - SOLID - AHA - Discover abstractions, don’t design with them. - Effective Go / language specific guidelines
40. 40 Confidential D1. Align with Design principles - DRY - SOLID - AHA - Discover abstractions, don’t design with them. - Effective Go / language specific guidelines NOTE: Guidelines, not rules to live by
41. 41 Confidential 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; }
42. 42 Confidential 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 ?
43. 43 Confidential 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 ?
44. 44 Confidential What does that even mean DM @kaustubh D3. N+1 query
45. 45 Confidential 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 }
46. 46 Confidential 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
47. 47 Confidential 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 ?
48. 48 Confidential 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 benefits over single method
49. 49 Confidential 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 benefits over single method
50. 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 }
51. 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 }
52. 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 }
53. 53 Confidential D5. Eager exits GetRecordByID(id string) { // checks record := getDBRecord(id) if record != null { // formatting return something; } else return null; }
54. 54 Confidential 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;
55. 55 Confidential 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;
56. 56 Confidential 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;
57. 57 Confidential Process improvements
58. 58 Confidential P1. Unit tests / Integration tests What does a unit test mean? Integration test ? Why are they really required ?
59. 59 Confidential 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 flow, but not this feature alone
60. 60 Confidential P1. Unit tests / Integration tests HTTP server Input Output Feature 1 Feature 2 Feature N
61. 61 Confidential P1. Unit tests / Integration tests HTTP server Input Output Feature 1 Feature 2 Feature N Unit test Integration test
62. 62 Confidential P2. Test your own changes ! • Install Dev App • Test PR on dev app
63. 63 Confidential P3. Prove you are right - Stackoverflow links - Go Playground - Official pkg docs Ref: https://github.com/
64. 64 Confidential P3. Prove you are right - Stackoverflow links - Go Playground - Official pkg docs Ref: https://github.com/
65. 65 Confidential 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
66. 66 Confidential 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, confirm your approach makes sense. - OK to explore as you go, but try not to delay release too much
67. 67 Confidential 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, confirm 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 🤮
68. 68 Confidential P6. Commit etiquette - Each commit should do exactly one task. - There is a sweet spot for a commit size.
69. 69 Confidential BAD - Implement feature - address feedback GOOD P6. Commit etiquette GOOD - [chore] / [add] / [fix] Depends on repo enforcements - “[CGT-123] add test for method” Shell Automations Debugging larger repos
70. 70 Confidential P7. Code can be neat
71. 71 Confidential P7. Code can be neat Ref: https://github.com/
72. 72 Confidential P7. Code can be neat How to do it ? Ref: https://github.com/
73. 73 Confidential P7. Code can be neat How to do it ? <details> <summary> Text </summary> Details </details> Ref: https://github.com/
74. 74 Confidential P7. Code can be neat How to do it ? <details> <summary> Text </summary> Details </details> There are tons of markdown features like this, please use them ! Ref: https://github.com/
75. 75 Confidential P8. Code review has a summary (save author’s time) Ref: https://github.com/
76. 76 Confidential 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
77. 77 Confidential Growing from a programmer to an engineer
78. 78 Confidential 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
79. 79 Confidential Improving as an engineer - Read tech articles - See how others do code review / Prepare their PR - After the first 2-3 months, see EVERY PR reviewed and merged recently - Good for feature education, decision making process
80. 80 Confidential Traffic CPU, mem DB Service cost User Inquiry Actual implementation Releasing a new backend feature
81. 81 Confidential 🚢 Devdojo: Ship code faster @kaustubh 2024.04.10 Questions