--- # Code Review<br> with Robot Kang-min Liu LINE Fukuoka ---  https://hackmd.io/@gugod/r13BV_FVr#/ --- ❓ 🖐 ㉄ --- # Me - Kang-min Liu / 劉康民 - https://gugod.org - Twitter: [@gugod](https://twitter.com/gugod) - Perl - Server-side dev ---- - [perlbrew](https://perlbrew.pl) - [Hijk](https://metacpan.org/pod/Hijk) - miminalist HTTP client - [LINE::Bot::API](https://metacpan.org/pod/LINE::Bot::API) - SDK for writting LINE Chatbot --- # 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 - `perl -c` - Perl::Critic / Perl::Lint - CPAN::Audit - eslint https://eslint.org/ - gofix https://golang.org/cmd/fix/ ---- ## 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. ```perl if ( grep { $_->can_code() } @people ) { say "Developers!"; } ``` ---- Better: just enough amount of work ```perl use List::Util 'first'; if ( first { $_->can_code() } @people ) { say "Developers!"; } ``` --- ## Prohibit Fixed String Matches ```perl my $is_product = $object =~ m/ \A (?: sticker | theme | emoji ) \z /xms; ``` ---- ```perl my $is_product = $object eq 'sticker' || $object eq 'theme' || $object eq 'emoji'; ``` ---- ```perl my $is_product = first { $object eq $_ } ( 'sticker', 'theme', 'emoji' ); ``` ---- ```perl use Quantum::Superposition 'any'; my $is_product = $object eq any('sticker', 'theme', 'emoji'); ``` ---- ```perl # 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](http://haya14busa.com/reviewdog/) Reviewdog を飼ってコードレビューや開発を改善しませんか  http://haya14busa.com/reviewdog/ ----  --- ## [perlcritic](http://perlcritic.com/) + [reviewdog](http://haya14busa.com/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](https://metacpan.org/pod/Devel::Mutator) Mutation Testing ---- Original ``` if (a && b) { do_this(); } ``` ---- Mutant ``` if (a || b) { do_this(); } ``` --- ## Wrap-up ---  --- 🖖 "Thank you for listening"; 🤟