Perform a code review

This is for:

Developer

This is a non-exhaustive list of questions you might like to ask yourself when performing a Quality Assurance (QA) review.

The code review is a key to the QA process. If you haven’t seen it yet, Qubit has some recommendations for performing QA when developing Qubit experiences.

  • Is PII exposed? Are any of the following hard coded or loaded in?

    • Email addresses

    • Names

    • User Ids

    • Any data that could identify a live person

  • Is there any logic in the experience that could cause the site to malfunction?

    • Any missing var/const/let keywords?

    • window.jQuery not used wherever possible

    • No assumed working code

    • No preview/debugging bypasses

    • Selector specificity checked, careful use of traversal methods, next, prev etc.

    • Check hides/removes are suitable (specific, non-recursive), check CSS for what’s being hidden

    • Are cookies suitably stringified, encoded/decoded?

    • Have you questioned anything you’re unsure of?

    • Are you carefully proxying any existing site functionality (avoiding where possible)?

  • Will the experience code have any unanticipated performance effects on the site?

    • Check for UV/QP declared before smartserve.js, do we need to poll? If sync must still be checked via an if, for example, if (uv && uv.transaction) { // do stuff }

    • Pollers only run when required

    • No sync requests

    • Everything on CloudFront/pngineer

    • Excessive data isn’t processed

    • Loops are sanity checked to prevent infinite calls and bailed when possible

  • Is there anything in the code that could cause a conflict with any on-site scripts or libraries?

    • Is everything scoped as you would expect, js, html, css

    • Will experience changes affect existing functionality/tracking, removal of event handlers etc?

  • Will this segment and trigger correctly, only firing when a change occurs, based on the logic defined? Could it cause a bad split?

    • Has the traffic split been checked?

    • Do segments attached to the experience make sense?

    • Will trigger logic cause a bad split?

    • Does the trigger make any changes to the control?

    • Have equality/regex tests been done in a non-case sensitive manner?

    • Is everything defined? For example, no dependence on $

    • Has everything been polled for correctly? are items passed through the poller rather than redefined? have selectors being passed to the poller been checked for quantity (for example, modifying 5th element from a group but only polling for the overriding class name in trigger)

    • Will the experience only fire when a change happens?

    • Does the experience match the brief?

    • If this experience has a lightbox component, should the exit feedback lightbox be disabled?

  • Is the use of APIs suitably engineered?

    • Does the code use a Deliver module or pull from deliver-lib where available?

    • Are responses cached where possible?

    • Are responses sanity checked and considerate of XSS attacks? (Stash/Tally in particular)

  • Is there anything in this experience that exposes any proprietary technology or systems?

    • Are there any passwords or keys exposed?

    • Any unnecessary technical detail commented in?

  • Will this experience function on the targeted devices?

    • Does the experience match the brief?

    • How does the experience react to window size changes?

  • Are there any areas of the code that are not cross browser compatible?

    • Will this work on IE9+?

    • What about safari?

    • Any concerns around in private browsing, for example, localStorage check?

  • Is code written in an efficient manner?

    • What could be done to reduce CPU time?

    • Check for code repetition?

    • Is jQuery necessary? (animations, expensive selectors)

    • Is there a suitable level of caching?

    • Any unused variables?

  • Does the code meet accepted industry-wide conventions?

    • Is the code in JavaScript standard style?

    • Is the code readable? are functions named indicatively of what they do?

    • Is there a sensible level of nesting?

    • Check for functions within functions?

    • JSON parse/stringify wrapped in try/catch?

    • Parse int/float wrapped in isNaN checks?

    • Is the code written in a functional manner?

    • Are comments provided for complex functionality?

    • Has anything been overcomplicated?

    • Is the code professional? Check for comments/function names?

    • Typos?

  • Tracking and events

    • Are all the events registered as goals?

    • Do the action names match up?

    • Have you seen the events go off in the network panel?

    • Are the event handlers bound correctly?