Adobe tech conference 2019

Adobe_Tech_Conf_2019.md

Code Reviews: Its not kindness, its diplomacy

Elodie Rafalimanana https://tinyurl.com/ydez8v94

The Problem

• developers argue about code during a code review
  • "It does not make sense, its a horrible idea, you're doing it wrong, its just cosmetics"
• causes inertia and animosity = inefficient
• What are they actually arguing about?
  • a pull request, one hnopeful small piece of code that will be merged into the great ocean of the code base
• how to communicate to a developer that their code needs to be changed, without generating hate / discomfort / pain

What is diplomacy

• conduct of international relations?

• Diplomacy is a technique, a social framework

• diplomacy has an algorithm

• can be implemented

• Definition: Diplomacy is a social framework used to achieve goals without making enimies

• Algorithm: Always choose what brings you closer to your goal, without hurting other people, use wisely

public class Diplomacy {
  // goal driven
  private final Goal goal;
  
  public Diplomacy(Goal goal) {
    this.goal = goal;
  }
  
  // action driven
  public Action getAction(Situation situation) {
    Action action = new Action();
    // compute action from sutation and goal
    ...
    return action;
  }
}

• Rules
  • Understand first: Everyone does something because they think it's the right thing to do
  • Every action contributes to the end goal
    • action needs to help build something

public class Diplomacy {
  ...
  public Action getAction(Situation situation) {
    Set<Actions> actions = getActions(situation);
    
    // from the understand rule
    understandFirst(actions, situation);
    
    // from the action that contributes to end goal rule
    Action action = getClosestAction(actions);
    
    return action;
  }
}

• remember to be kind
  • hard to enact in a stressful deadline environment

How to handle

• "your code doesn't make sense"
  • "F*** you!"
  • "please, be kind"
  • "I did this to hanle the null value"

Code Review

• As the reviewer
  • Goal: merge good code (without making enemies)
  • Actions:
    • Ask for a change
    • ask for an explanation
  • consider reviewee considers the code finished, and is impatient to merge
    • reviewer is likely resistant to change
  • respect the work done, help the reviewee
  • use language that enhances, enables, and encourages
    • people do not like being told what to do or undermined

Technique Enhance

• something to improve, but is not bad
  • Example: suggestion for renaming something
    • Use positive comparatives
    • more + readable, maintainable, efficient, performant, robust, clear, consistent, resuable, debuggalge, etc
    • Use open suggestions (leave the reviewee a choice)
    • can this be...?, what do you think of...?, is it possible to...?
      • I suggest to..., this would be more..., you could do it like this...
  • Example: "Can this be factorized?" NOT "Too much duplicated code :)"
  • Example: "This can be remoed." NOT "useless"
  • Example" "It would be more readable named as..." NOT "This method name is confusing"
• What if there are required changes: Bugs, feature requirements, performance
  • ask for change clearly and remain neutral
  • use a passive voice
    • "It should be, needs to be, must be, hast to be fixed"
  • use "we" to place yourself in the same boat as the reviewee, brings them into the team / part of the larger code base
    • "We should, have to, must, need to fix this"
  • use the polite form
    • "Could you please fix this?"
  • Example: "This needs to be checked" NOT "You should check this"
  • Example: "We should check this" NOT "You have to check this"
  • Example: "Could you please check this?" NOT "Check this"

Enable technique

• maintaining the motivation of the reviewee to apply the changes, stop disabling
• 7 dont't
  • don't assume, understand first
    • NOT "You're wrong", instead: "can you explain why we need this?", "Is this intended?", "Is there a particular reason why?"
  • don't judge, here to ask for change or an explanation, this is not an evaluation
    • NOT "Your code is bad / ugly / wrong / clumsy", instead "This needs to be changed like this"
    • NOT "I don't like this pattern, I prefer this one", instead "I suggest this pattern"
      • remove personal preference
    • Do not use evaluative descriptors: Good, bad, great, right, wrong, ugly
  • don't point, do not accuse
    • NOT "you forgot to import this file", instead "this file should be imported"
  • don't give orders, this is a PEER review
    • NOT "use this library :)", instead "we can use this library instead, it's more efficient"
  • don't replace, instead transform what's done
    • careful with code snippets, your code is not here to replace theirs, but is a suggestion
    • even so, do with care
  • don't be aggressive, be calm
    • dont use capital letters, offensive words, upset emojis or exclimation points
  • Don't humiliate
    • send bugs / stack errors privately first
    • check that it is an actual error and not related to your environment
    • leave steps to reproduce and reference on PR
    • "I tested with this value and it trigered an error"

Encourage Technique

• motivate the reviewee
• must learn to negotiate
• give action driven feedback
  • NOT "It seems not optimized", instead "It can be optimized by using Map.entry"
• Argument driven feedback
  • NOT "It should be renamed", instead "It should be renamed because it would be clearer"
• persist politely
  • the reviewee will likely make excuses to not make the suggested changes, remember to persist kindly

Conclusion

• A general guide to giving a code review comment:

(1) it should be + (2) checked + (3) because it's more + (4) robust.

1. Human resistance canceller

• passive voice / use "we"
• We should / we need to / it needs to

2. Action

• what needs to be done
• moved / fixed / renamed

3. Positive comparative

• improvement phrase

4. Argument

• reason why to apply change
• readable / maintainable / consistent

The Art of Software Design

Good vs Great?

• some would say a great coder is someone who has a solution and implements it quickly within the code base
• someone who's code enhances the productivity of everyone who encounters it
  • must write good code

What is bad code?

• Forest of Best Best Practices
  • DRY, dependency-injection, modularity, open/closed principle, dependency inversion principle, etc...
• Are those best practices really good code?
• Example: what does this do?

function f(a) {
  const b = [];
  for(let c = 2; c < a; c++) {
    let d = false;
    for(let e = 2; e < Math.round(Math.sqrt(c) + 1); e++) {
      if (c % e === 0) {
        d = true;
        break;
      } else b.push(c);
    }
  }
  return b;
}

• no documentation
• variable names are meaningless
• complex with imbedded loops, control flow, a break statement
• d variable serves no purpose but to guide control flow
• opaque expressions, unsure of what role they play in the function, what values they are calculating
• rigid and fragile
  • code modifications would need to be very careful

What is good code?

• respond rapidly to changing requirements
  • true test of code is how easy it is to change it
• good code is flexible and clear
  • easy to change, does not require changes in multiple places
• documentation, independence, simplicity
  • independence means changing one function / data structure does not require changing another
  • documentation means writing in plain english what the code does / why
  • simplicity means reducing cognitive overhead
• Example:
  • Add documentation:

// this file defines functions related to testing primality and finding prime numbers
// top-level function is primes()
function primes(n) {
  // param n: a natural number
  // returns a list of the primes less than n
  const result = [];
  for(let candidate_prime = 2; candidate_prime < n; candidate_prime++) {
    // determine whether candidate_prime has any divisors
    let has_devisor = false;
    for(let candidate_divisor = 2; candidate_divisor < Math.round(Math.sqrt(k) + 1); candidate_divisor++) {
      if (candidate_prime % candidate_divisor === 0) {
        has_devisor = true;
        break;
      // candidate_prime has no divisors, so it is prime - add it to the list
      } else result.push(candidate_prime);
    }
  }
  return result;
}

• make independent:

// this file defines functions related to testing primality and finding prime numbers
// top-level function is primes()
function primes(n) {
  // param n: a natural number
  // returns a list of the primes less than n
  const result = [];
  for(let k = 2; k < n; k++) {
    if (is_prime(k)) result.append(k);
  }
  return result;
}
  
  // separate implementation from intent
function is_prime(k) {
  let has_devisor = false;
  const highRange = Math.round(Math.sqrt(k) + 1);
  for(let d = 2; d < highRange; d++) {
    if (divides(d,k)) {
      has_devisor = true;
      break;
    }
    return !has_devisor;
  }
}
  
function divides(d, k) {
  return k % d === 0;
}
  

• simplify?:

// this file defines functions related to testing primality and finding prime numbers
// top-level function is primes()
function primes(n) {
  // param n: a natural number
  // returns a list of the primes less than n
  const result = [];
  for(let k = 2; k < n; k++) {
    if (is_prime(k)) result.append(k);
  }
  return result;
}
  
  // separate implementation from intent
function is_prime(k) {
  let has_devisor = false;
  const highRange = Math.round(Math.sqrt(k) + 1);
  for(let d = 2; d < highRange; d++) {
    if (divides(d,k)) {
      has_devisor = true;
      break;
    }
    return !has_devisor;
  }
}
  
function divides(d, k) {
  return k % d === 0;
}

Overview

• documentation
  • document every file, class, method. Use self documenting names
  • wrap up code blocks into methods that represent important concepts
  • simplify code with high complexity. It should read so clearly that its intent is immediately obvious just by looking at structure and names of methods it calls.

1 frontend dev, 50 backend devs: How a design system helps

Toby Dunkelberg

Ethos

• use onboarding with OrCA
  • only one UI engineer
• deploy with moonbeam
• both use rails and Bootstrap
• both were not DRY
• inconsistent UX across UI

release process

• two different possible paths
• feature request -> design work -> implementation -> release
• OR -> rough implementation -> release -> design work -> update & re-release
• would create inconsistent experiences
• huge bottleneck on UX dev

How to scale UI/UX work

• create a design system
  • collection of principles, design patterns, and documentation into a toolkit
• UI patterns & reusable code components that meet high standards
  • Google Material
  • Adobe Spectrum
  • Bootstrap

Toolkit

• principles
• documents
• guidelines

pros and cons

• Benifits
  • enforce high standards
  • enables all devs to contribute
  • speeds up UI development
  • single source of truth
• cons
  • time intensive to learn
  • requires to switch to different styles

Ethos design system

• packaged Ethos components in one locatoin
• available as Ruby gem and precompiled assets
• demo website for components
• consistent use cases

rules

• stop adding to the problem
  • dont create more within backlog
• work component by component
  • when receieve new feature request, work on it yourself until it is good to go
  • trust within system that it will work
• empower the team to help
• make new designs known

new process with stardust system

• feature request -> create new component -> add to stardust system -> update view
• OR -> update view if components already exist

whats next

• use spectrum
• shift toward spectrum

SledgeHammer - Distributed performance testing as a service

Marius Loana & Ionut-Maximum Margelatu

Present day testing

• testing is more difficult as products are now a conglomeration of many services

Specs

• geo-distributed across multiple areas of the world
• REST API
• high throughput real-time stream processing using Kafka
  • 100,00 events / s
• low latency in house Java SDK
  • 45,000 rps

Choices

• need load tests for various inputs
• easy to extend and create new load generator types
• support for running different types of tests at same time with same infrastructure
• support for customizing client side metrics for measuring throughput, latency
• easy to start / stop
• metrics dashbaord

sledgehammer

• distributed
• support for many different servies (Kafka, REST, etc.)
• Scales in Ethos or Kubernetes cluster
• tests are expressed as Java code
• easily extendiable (needs work)
• custom dashboards for monitoring tests
• pleasant UI

Testing as a product - Enabling Cross solution tests

Baubak Gandomi

packaging

• many roles in the org should be interested in testing

Participation

• Execution
  • runtime parameters
  • run control
  • access to jenkins jobs
• reporting
  • logs
  • results
  • clear assertions
  • make results available to other people
• Documentation
  • JavaDoc
  • Wiki
  • Release Notes
  • good comments
• Qualitu
  • test libraries
  • test the tests!
  • CI/CD
  • SDLC
  • think in release cycles of testing

Design

• End to End scenarios
• Cross solution test sharing
• Force everyone to use the same testing library? No
• Copy / paste tests into each area of the app? does not scale

Test as a product

• your test is a product
• can share as a library
• engineers will use libraries (Integro)
• separate tests and manipulators
  • dont want to empose a certain framework
  • manipulators are best to share
  • allows for end to end

Dancing with Dragons; Quest for Modern JS Development

Michael Smith

Planning for the future

• Angular was pretty popular in 2014, was backed by google
• slowed down by tech debt in 2018
  • angular, grunt, require.js
  • modernize ES6, react, jest, webpack
• add new features in new tech
  • start with foundations: language, build system, testing framework
    • most distruptive
  • then move to state / state mamagement library
  • UI: JS framework, UI components, CSS/styling
• switch to webpack - undstands most module systems
  • transpile with Babel to use modern JS
    • target specific broswer versions
• no one can predict the next web framework
  • yet JS has matured quite a bit
  • not as much change as of late
  • attempt to structure code as best you can

Unit testing

• should allow you to refactor without breaking
• but if you change the unit test framework?
  • rewrite tests after code?
  • same time? no safety net
• start from scratch instead with Jest
  • have required coverage amounts
  • use data-test-ids

model and view

• use mobex for state management
• had state stored in services in Angular
• gradually convert to ES6 classes and add tests
  • bottom up, simple first
• injected angular services into converted code for the in-between time

State Management

• everything should be a function of the state (dont store derived state)
• use cachert if that function is costly
• make the asynchronous synchronous!
  • getValue() will store a cached value, if cached value does not exist, fetch it and appear loading, then update the catch

UI

• conerting angular to react
• do it in stages by injecting react into the angular app
  • middle down approach
  • avoids fragmentation
  • can focus on a self-contained part
  • add a feature flag while its being worked on, after finished remove feature flag
• there is always a way of injecting one framework into an other

tips

• start with dev environment
• gradually convert code, bit by bit
• ensure high standards (code review, paradigmns, test coverage)
• new features written in new framework
• reafactor and simplify!
  • dont keep old nasties

Masonry for Smart Layouts

Smriti Singh

masonry layout

• print media is a good resource of masonry
• good for media libraries, portfolios, blogs
• adopted by:
  • pinterest
  • the hunt: variable heights and widths
  • adobe stock: fixed width, variable height
  • tumblr

how hard?

• like a jigsaw, but harder
• many possible fits, must find best fit out of many possible
• ranking must sometimes be sacrified for white space opimization
• added images to Adobe Color

what can help

• quick layout, responsive to screen size, can do virual scrolling, ranking, filtering, smooth rendering
• virtual scrolling only renders dom nodes that are visible

Choices

• Packery
  • quick layout, responsive
  • NO filtering, ranking, virtual scrolling
• Masonry
  • quick layout, responsive, ranking
  • NO filtering, virtual scrolling
• Isotope
  • quick layout, responsive, ranking, filtering
  • NO virtual scrolling, multiple instances
• Bricks.js
  • quck layout, responsive (not infinite), ranking, multiple instances
  • NO filtering, virtual scrolling
• pinterest
  • quick layout, responsive, ranking, virtual scrolling, multiple instances
  • kind of filtering

One size does not fit all

• pintrest does not work well for some of the other needs like draggable, variable widthds

what to look for

• one variable dimensions at a time, saves time
• update, don't layout (take out nodes updated, not everything)
• vituall scroll will get you there, smoothly
• keep it light with changing the DOM tree

other ideas

• CSS grid and flex is still pretty good
• consider moving from infinite scroll to pagniation
• virtual scroll may help
  • react-virtualized
  • Adobe collectionView

##Performance in JS Joe Stachowitcz github.com/Adobe-marketing-cloud/ sitespeedio - for measuring speed github: reactor

Mixed JS solutions

• had many solutions for the same problems in the UI
  • formed a unified JS team

components of web performance

• deconstruct existing libraries
  • get into fine details
• reduce code duplication
  • multiple read / write cookies
• implement modern coding practices
  • JS moves quickly
• change build procedures

lab results

• created a unified lab to test all solutions with benchmarks
• Adobe Exerience Platform Auditor was created
  • rebrick of unit tests that grades a site

reality

• customers would come back with Google's pagespeed and lighthouse did not match adobe's tests
• customers have the idea that performance should match Google search page

Definition Gap

• disagreement
  • browser paint
  • load
  • render
• agreememt
  • response times
  • DNS lookup

Device issues

• 2G & 3G are dominant in emerging mobile markets
• 4G / LTE are available in most established markets, but average practical speeds are 3G
• 3G is target group
• we are reporting at 4G speeds

Regional Cache Change

• had a 61% first paint improvement
• 78% first visual improvement
• use most current adobe code
  • integration advancements
  • bug fixes
  • security

Launch

• takes extension code for each solution
  • analytics
  • target
• pools shared pieces
• configure
• run merged custom build.js

lessons

• stay current
  • on top of education
• frameworks are not always the answer
• out with the old
  • if you see global browser usage (<3%) drop it from supported
• eliminate code repitition
• refactor code to make lighter
• diligence in reviewing your processes
• speed is not the only thing, but is primary thing
• some tools for testing are better than others

Getting Browser Bugs Fixed

Alan Stearns

log bugs!

• one less workaround you have to deal with
• helping other web engineers
• log a bug, then vote on a bug browser-issue-tracker-search.appspot.com

improve chances

• add a test to your log
• repeatable test steps
• add a cross browser test
• web-platform-tests.org
  • repository of all cross browser tests
  • synced bi-laterally with all broswers
  • run independently on all broswers
  • will make it easier for browser devs to repo and fix

How to write?

• web-platform-tests.org/writing-tests/index.html
• testharness.js
  • simple like unit tests for js
• Reftests
  • a bit more complicated
• ignore other things
• use presenter to talk to browser engineer for you

Accessibility

• michael fordan and James Nurthen (presentation on thumbdrive)

Example: Date picker

• with a mouse, works as expected
• with a keyboard user has to navigate through any other interactive controls to reach popover
• pop over is rendered out of context
• inaccessible to screen reader user

Key Principles

• keyboard operability
• visibility of the focus indicator
• predictibility of movement
• persistence of focus
• maximize efficiency
  • minimize number of navigation steps
  • consistently apply keyboard interface conventions for composite controls

Example: Date picker revisited

• calendar receives focus when popover opens
• popover tabs to focus
• calendar control minimizes number of navigation steps to select a date using arrow keys to navigate table
• closing popover restores focus to date picker

Elements that can receive focus

• interactive controls native to HTML
• any element with a [tabindex]
  • 0 = add to tab sequence in order it appears in the DOM
  • -1 = remove from tab sequence, but still focusable

  • 1 = avoid, removes from logical order of the DOM

• element must be:
  • visible in the dom
• composite controls have more than one level of interactivity
  • combobox
  • grid/table
  • select list
• designers should design UX / interaction with keyboard
  • dont want a user to tab through the entire list of thousands of options

Roving tab index

• navivation keyboard event changes tab index
• FocusManager: Roving tabIndex in react spectrum

Aria-activedescendant

• when composite component receives keyboard focus
  • aria-active descendant="IDREF"
  • focus styling msut be explicitly applied to focused descendant

virtual scrolling

• render only enought item views to fill the dataport
• challenges
  • maintain DOM order
  • focus lost when item views recycle (unless managed)
  • poor candidate for aria-activedescendant
• solutions
  • correct item order after scroll
  • manage focus as a state, distinct from item selection
  • focus container element when item focus is scrolled out of view
  • restore item focus ASAP on scroll or animation complete
  • make descendants tabable on item focus

keyboard accesible drag and drop

• use roving tab index for efficient navigation
• drop target is tabbable and follows focused page
• move drop target within grid using arrow keys
• focus is maintained when drop target is moved
• maintain focus after move operation
• keyboard is hit for on / off selection, does not need to continually be held