Review

ReviewChecklist.md

Development

sequenceDiagram
    participant master
    participant feature
    master->feature: clone local and push to origin
    loop commit
        feature->feature: commit 1 ...
        feature->feature: commit 2 ...
        feature->feature: commit 3 ...
    end
    feature->test  : merge,deploy and ask for review!
    Note left of test: feedback from reviewer as comments in gitlab!
    feature->master: merge and deploy live!

Loading

- use php-storm plugins to follow coding-standard and guideline - keep features/code reviews small (max 200LOC) - commit early and often

one redmine-ticket => one feature-branch: [JJJJMMTT]_[#12345]_[lastname]_[the-description-of-the-feature]

"Any fool can write code that a computer can understand. Good programmers write code that humans can understand. " __Martin Fowler

Functional Test (5 - 10min)

When

• a clear UserStory with all feature-requirements are given
• feature is merged to test, deployed and tested by feature-developer / before merging to master

How to test

• check whether the requirements were implemented from the user's point of view (Frontend/Editor)

+ Review (10 - 20min)

How to review

• use php-storm diff-tool (diff to master) to see all customized files
• only developer makes changes after your feedback (knowledge sharing)
• do not push reviewers logic to feature-branch (review != refactoring)
• if no context to project = review as pair (feature-developer / reviewer)
• use comment tiers for your feedback:

comment tiers	comment	deployment
you might	better comments, ubiquitouseLanguage, KISS	O (yes)
you should	CGL, DRY, YAGNI, SRP	O (yes)
you must	bug, insecure, lack of validation	X (no)

Checklist

• SRP (Single Responsibility Principle)

  • class-function ~10 Lines of Code
  • class should have only one reason to change

• KISS (Keep it simple, stupid)

  • stupid functions don't know about referrer/environment

• DRY (Don't repeat yourself)

  • use template-partials to prevent duplicate markup
  • use functions to call multiple code-snippets

• CGL (PSR -CodingGuideLines)

  • camel-case-written,readable functions and variables
  • domain-driven ubiquitous Language

• COMMENT

  • why we use this code-constuction?

• REQUIREMENT

  • meet all requirements of the user-story
  • which part of code solves an reqirement ?

• FUNCTIONAL

  • the code runs without errors
  • the feature shows the desired behaviour

• YAGNI (You Aren't Gonna Need It)

  • should not add functionality until deemed necessary
  • should integrate existing libraries before reinventing the wheel

+ Refactoring (30+Xmin) - as pair