Code Review
with Robot

Kang-min Liu
LINE Fukuoka

https://hackmd.io/@gugod/r13BV_FVr#/

❓ 🖐 ㉄

Me

My team

LINE Creator Market

https://creator.line.me/en/

  • 3 frontend devs

  • 4 server-side devs

  • frontend: angularjs → vue.js

  • server: perl, mysql, nginx, memcached

Flow of work

  • GitHub Enterprise
  • New branch for tasks
  • git push → CI
  • Code Review before merging
  • Human QA before releasing

Code Review

  • First task of my day
  • about 30% office time

Pros

Gain understandings on

  • Code
  • Spec
  • Why
  • How

Focus

  • Readibility > Correctness

  • Maintainability > Optimized solution

  • Unconventional

    • prefer tech X for problem Y

  • Quirks, magic, shenanigans

    • "Why does it need to be done like this?"

  • Mismatching style

    • "This does not looks pretty"

  • Random "code waste"

    • Unused, effect-less, NOOPs

Can I understand this new piece code
written by my teammate
by just reading it ?

understandable code →→→ maintainable code

Tools to swipe code wastes

  • Formatter (re-indent / tweak whitespace)
  • Linter
  • Tester
  • Refactor-er (Mutation + Test)

Formatter (re-indent / tweak whitespace)

  • perltidy
  • $EDITOR

Linter

Tester

  • CI

  • Continuous Testing (Test::Continuous)

  • All pair testing

  • Mutation Test (Devel::Mutation)

  • fuzzing

Refactor-er

  • App::Critique (semi-manual)
  • Ref::Util::Rewriter

Formatter

  • Dress
  • "Fashion"
  • Shape & Color
  • Whitespace management

Note:
Why using "LaTeX" instead of $whatever ?
How many time you've spent on configuring the fonts & color of your Termnial / Editor?
You probably dislike Comic Sans font for a reason.

  • $x spaces
  • $y tabs
  • $z newlines
  • Talk this though with some drinks

$EDITOR

Linter

  • Identify wastes
    • Unused variables
    • Unused subroutines
    • Unused imports
    • Unused hash key/value
  • Uncleaned namespace

Perl::Critic

Critique Perl source code for best-practices.

Perl::Critic is a static code analysis system for the Perl programming language. Perl::Critic is available as a source-code distribution on CPAN. It comes with a commandline tool, perlcritic, which can check Perl source code files and report on the code quality therein. Perl::Critic has an extensible architecture that allows the programmer to choose from many "policies" which enforce different Perl programming styles and tastes.

http://shop.oreilly.com/product/9780596001735.do

Online Demo

http://perlcritic.com/

Prohibit Boolean Grep

Bad: too much work.

if ( grep { $_->can_code() } @people ) {
    say "Developers!";
}

Better: just enough amount of work

use List::Util 'first';

if ( first { $_->can_code() } @people ) {
    say "Developers!";
}

Prohibit Fixed String Matches

my $is_product = $object =~ m/
  \A (?: sticker | theme | emoji ) \z
/xms;

my $is_product = $object eq 'sticker' ||
                 $object eq 'theme' ||
                 $object eq 'emoji';

my $is_product = first { $object eq $_ } (
  'sticker', 'theme', 'emoji'
);

use Quantum::Superposition 'any';

my $is_product = $object eq any('sticker', 'theme', 'emoji');

# perl6
my $is_product = $object eq 'sticker'|'theme'| 'emoji';

Perl::Critic::TooMuchCode

  • ProhibitUnusedConstant
  • ProhibitUnusedImport
  • ProhibitUnusedInclude
  • ProhibitLargeBlock
  • ProhibitLargeTryBlock

https://metacpan.org/pod/Perl::Critic::TooMuchCode

Code-fixable patterns

  • do_this();; Extra semi-colons / empty statements.
  • Unused hash key/value
  • typo in Document
  • typo in variable name (maybe)

CI

  • Jenkins

  • Travis

  • CircleCI

  • Drone CI

Code Review in CI

reviewdog

Reviewdog を飼ってコードレビューや開発を改善しませんか

http://haya14busa.com/reviewdog/

perlcritic + reviewdog

Let CI do some code reviews

# .reviewdog.yml
runner:
  perlcritic:
    cmd: perlcritic --profile .perlcriticrc --verbose 1 *.psgi lib/
    errorformat:
      - "%f:%l:%c:%m"
    name: perlcritic

perlcritic + reviewdog

https://hub.docker.com/r/gugod/perlcritic-reviewdog

Usable in Drone CI.

CPAN::Audit

Scan dependency graph for
known vulnerabilities

Devel::Mutator

Mutation Testing

Original

if (a && b) {
    do_this();
}

Mutant

if (a || b) {
    do_this();
}

Wrap-up

🖖 "Thank you for listening"; 🤟

Select a repo